Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updgrade to the latest versions of AppAuth-JS. #3

Merged
merged 2 commits into from
Jan 29, 2019

Conversation

pixtron
Copy link
Contributor

@pixtron pixtron commented Nov 9, 2018

  • upgrade @openid/appauth to ^1.1.1
  • upgrade electron to ^3.0.8
  • upgrade typescript to ^3.1.6

* upgrade @openid/appauth to ^1.1.1
* upgrade electron to ^3.0.8
* upgrade typescript to ^3.1.6
@mraible
Copy link

mraible commented Nov 17, 2018

Are there plans to merge this in the near future?

@tikurahul
Copy link
Collaborator

Just realized that here was an outstanding PR against the sample repo.
Thanks for your PR. Let me take a look.

flow.ts Outdated
@@ -83,7 +84,13 @@ export class AuthFlow {
this.notifier.setAuthorizationListener((request, response, error) => {
log("Authorization request complete ", request, response, error);
if (response) {
this.makeRefreshTokenRequest(response.code)
let codeVerifier: string | undefined;

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nit: Remove unnecessary line breaks.

flow.ts Outdated
@@ -132,19 +139,27 @@ export class AuthFlow {
);
}

private makeRefreshTokenRequest(code: string): Promise<void> {
private makeRefreshTokenRequest(code: string, codeVerifier?: string): Promise<void> {
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Given that you are always passing codeVerifier a better type would be codeVerifier: string|undefined

@pixtron
Copy link
Contributor Author

pixtron commented Jan 29, 2019

@tikurahul sorry i saw your comments, but then got distracted by something/someone and just came back today. Changed the PR based to your review.

@tikurahul tikurahul merged commit 178605a into googlesamples:master Jan 29, 2019
@pixtron pixtron deleted the update-appauth branch December 5, 2019 21:52
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants