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

Azure Active Directory Auth: Adding try-catch block on callback to capture empty key #2010

Merged
merged 6 commits into from
Mar 16, 2021
Merged

Azure Active Directory Auth: Adding try-catch block on callback to capture empty key #2010

merged 6 commits into from
Mar 16, 2021

Conversation

jeliasson
Copy link
Contributor

As per ongoing partial Azure Active Directory outage I noticed that the error message thrown by the azureActiveDirectory auth client might be enough for initial troubleshooting.

Error

api | TypeError: Cannot read property 'getPublicKey' of undefined
api |     at Immediate._onImmediate (/home/johan/work/git/sjoraddningssallskapet/ssrs-se-portal/node_modules/@redwoodjs/api/dist/auth/decoders/azureActiveDirectory.js:39:33)
api |     at processImmediate (internal/timers.js:463:21)
api |     at process.callbackTrampoline (internal/async_hooks.js:131:14)

This PR will give the developer an early indication on what may be wrong.

api | An error occured while trying to obtain signing key from Azure Active Directory. This might be a result of a Azure Active Directory outage. See https://status.azure.com/en-us/status for current status.
api |  TypeError: Cannot read property 'getPublicKey' of undefined
api |     at Immediate._onImmediate (/home/johan/work/git/sjoraddningssallskapet/ssrs-se-portal/node_modules/@redwoodjs/api/dist/auth/decoders/azureActiveDirectory.js:39:33)
api |     at processImmediate (internal/timers.js:463:21)
api |     at process.callbackTrampoline (internal/async_hooks.js:131:14)

@jeliasson
Copy link
Contributor Author

@dthyresson Could you please review at your convenience.

@dthyresson
Copy link
Contributor

@jeliasson Seems fine to me - do you need the extra \n in the console?

@Tobbe any issues here you can see?

@jeliasson
Copy link
Contributor Author

jeliasson commented Mar 15, 2021

@jeliasson Seems fine to me - do you need the extra \n in the console?

Not really. Just looked prettier but I agree it should not be there, and I will remove it. :)

@dthyresson dthyresson requested a review from Tobbe March 15, 2021 21:28
@github-actions
Copy link

github-actions bot commented Mar 16, 2021

📦 PR Packages

Click to Show Package Download Links

https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/create-redwood-app-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-api-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-api-server-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-auth-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-cli-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-core-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-dev-server-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-eslint-config-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-eslint-plugin-redwood-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-forms-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-internal-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-prerender-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-router-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-structure-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-testing-0.27.1-0b338ef.tgz
https://rw-pr-redwoodjs-com.s3.amazonaws.com/2010/redwoodjs-web-0.27.1-0b338ef.tgz

Install this PR by running yarn rw upgrade --pr 2010:0.27.1-0b338ef

Ty @Tobbe

Co-authored-by: Tobbe Lundberg <[email protected]>
Copy link
Contributor Author

@jeliasson jeliasson left a comment

Choose a reason for hiding this comment

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

I started a review, and I have no idea how to leave it. I guess I make a note here, and try to close it afterwards.

Edit: Uhm... I guess not. 😅

@Tobbe Tobbe merged commit 7e44304 into redwoodjs:main Mar 16, 2021
@jeliasson jeliasson deleted the jeliasson-azure-ad-auth-gracefully-handle-invalid-signing-key branch March 16, 2021 13:19
@jeliasson
Copy link
Contributor Author

ty @Tobbe and @dthyresson

@thedavidprice thedavidprice added this to the next release milestone Mar 16, 2021
dac09 added a commit to dac09/redwood that referenced this pull request Mar 22, 2021
…r-rwt-link

* 'main' of github.com:redwoodjs/redwood: (26 commits)
  Router: Fix auth (redwoodjs#2038)
  Steps towards a11y for Redwood Router (redwoodjs#1817)
  Router: <Set wrap={}> (redwoodjs#1898)
  Pass event and context into getCurrentUser (redwoodjs#1908)
  Implement Redwood API side Logger (redwoodjs#1937)
  Fixed path on windows to allow for pages under subdirectories (redwoodjs#2022)
  Add experimental ESBuild for api side (redwoodjs#1948)
  Upgrade to Prisma 2.19.0 (Prisma Migrate GA) (redwoodjs#2021)
  Fix lint breaking when deleting a side (redwoodjs#2017)
  Refactor: Converted Prisma.ts to js (redwoodjs#1958)
  Fix issue with verify email redirect using Auth0 (redwoodjs#1990)
  add GitHub Action CodeQL Analysis (redwoodjs#1951)
  fix: correct var name for grabbing user config (redwoodjs#2002)
  Create functions to fs calls (redwoodjs#2007)
  Return signup Output (redwoodjs#1992)
  Add makeExecSchema options (redwoodjs#1964)
  upgrade gotrue-js to 0.9.29 (redwoodjs#2011)
  Azure Active Directory Auth: Adding try-catch block on callback to capture empty key (redwoodjs#2010)
  withCellHOC: Fix TS error (redwoodjs#1967)
  Update error message in tasks/publish-local to point to tasks/run-local-npm when Verdaccio isn't running (redwoodjs#2004)
  ...
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants