[SDK-1715] Configuration and API updates #109
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
panva
suggested changes
Jul 7, 2020
lib/config.js
Outdated
| scope: Joi.string().optional().pattern(/\bopenid\b/, 'contains openid').default('openid profile email'), | ||
| response_mode: Joi.string().optional().when('response_type', { | ||
| is: 'code', | ||
| then: Joi.valid('query', 'form_post').default('query'), |
There was a problem hiding this comment.
when code, leave undefined (i.e. no default) to omit the parameter from being sent, all servers are required to respond with a query automatically. For all other response types we send the value form_post explicitly because it's not the default to be used.
lib/config.js
Outdated
| }) | ||
| } | ||
| ).when( | ||
| Joi.ref('authorizationParams.response_type', { adjust: (value) => value && value.includes('id_token') }), |
There was a problem hiding this comment.
idTokenSigningAlg being HS* is the requirement for needing a clientSecret, the response_type does not need to include id_token at all. ID Token will still be returned by the token endpoint with an HS signature even when response type is code. I believe the code before was correct and needed no changing (aside from the improved wording of the error message)
There was a problem hiding this comment.
Ah my bad, I thought that was a bug - will revert
There was a problem hiding this comment.
After our conversation I changed the behaviour to: clientSecret is required if idTokenSigningAlg is HS* regardless of response_type
panva
reviewed
Jul 7, 2020
Co-authored-by: Filip Skokan <panva.ip@gmail.com>
panva
approved these changes
Jul 9, 2020
adamjmcgrath
added a commit
that referenced
this pull request
Sep 17, 2020
* Update beta version * Initial commit of Beta 2 branch Co-authored-by: Filip Skokan <panva.ip@gmail.com> * Add PKCE tests and fix multiple servers in tests (#110) * [SDK-1714] [SDK-1723] Session cookie checks (#111) * Refactor the tests a little to use a server * simplify test * tests for unordered cookie chunks * Test format changes for transient handler * Update lib/transientHandler.js Co-authored-by: Filip Skokan <panva.ip@gmail.com> * [SDK-1715] Configuration and API updates (#109) * Simplify the config tests, test more with less code * Validate config fixes and tests * Add comment, update tests * `Issuer.discover` only takes a fully qualified url * Simpler scope assertion and keep all config tests in same file * Let auth server set default for response_type: code * clientSecret is required for HS* algs regardless of response_type Co-authored-by: Filip Skokan <panva.ip@gmail.com> * [SDK-1712] Test token set (#108) * Add tests for TokenSet * Refactor the tests a little * Split up code flow tests * Add tests for access token expiry * Add Prettier for style formatting (#112) * Add prettier * Run `prettier --write .` * [SDK-1716] Add tests for claim* MW (#113) * Add tests for claim* MW * Fix tests * Add some tests for session duration behaviour * Revert "Add some tests for session duration behaviour" This reverts commit e3ce510. * Add some tests for session duration behaviour (#114) * Add test for passing custom param to logout (#115) * [SDK-1721] Auto generated docs (#117) * Auto generated docs with typedoc * fix lgtm * Fix auth params * Fix incorrect import of `requiresAuth` (#118) * Add TROUBLESHOOTING and update debug logging (#120) * Add TROUBLESHOOTING and update debug logging * fix tests * Update lib/context.js Co-authored-by: Filip Skokan <panva.ip@gmail.com> Co-authored-by: Filip Skokan <panva.ip@gmail.com> * attemptSilentLogin feature (#121) * attemptSilentLogin feature * Resume silent login after successful login so that users try silent login again after their session's expire * `postLogoutRedirectUri` isn't always a URI and login needs a check for `response_type` (#123) * [SDK-1877] Add refresh method to access token (#124) * Add refresh method to access token * Add method to fetch userinfo * Add refresh example to docs * Not all refresh token grants get a new refresh token back (eg non rotating) and remove unneeded opts * Scope all cookies (skipSilentLogin, transient and appSession) to the app session cookie path and domain config (if specified) (#125) * [SDK-1722] Architecture (#128) * Default Login flow docs * Add logout * add link from readme * [SDK-1876] [SDK-1726] Add samples and smoke tests (#127) * Add samples and smoke tests * Fix CI * Make the test clearer that discovery alg is ignored (#130) * Make the test clearer that discovery alg is ignored * Add test to show "none" disallowed for idTokenSigningAlg * Disallow "none" in any case (#131) * Disallow "none" in any case * Fix puppeteer in CircleCI https://github.com/puppeteer/puppeteer/blob/main/docs/troubleshooting.md#running-puppeteer-on-circleci * Revert "Fix puppeteer in CircleCI https://github.com/puppeteer/puppeteer/blob/main/docs/troubleshooting.md#running-puppeteer-on-circleci" This reverts commit c0fd31e * use active lts * [SDK-1914] Add a migration guide for v1 to v2 (#129) * Add a migration guide for v1 to v2 * Apply suggestions from code review Co-authored-by: Filip Skokan <panva.ip@gmail.com> * Updates per PR comments Co-authored-by: Filip Skokan <panva.ip@gmail.com> * Release 2.0.0-beta.0 (#132) * Release 2.0.0-beta.0 * Ignore docs from lint * Fixes the numbering on examples (#136) * chore: update jose and openid-client (#134) Co-authored-by: Filip Skokan <panva.ip@gmail.com> Co-authored-by: David Patrick <david.patrick@auth0.com>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add some tests, finish off TODOs and verify the changes to the Configuration
openidalways in scopeclientSecretis required forcode id_tokenas well ascodewith HS* signing algsissuerBaseURLcan't be a hostname becauseopenid-clientIssuer.discoveryonly takes fully qualified URLsnullorfragmentinresponse_typebecause the callback handler doesn't acceptfragmentqueryforresponse_typecodebecause you don't need to protectcodewithform_postconfig.test.jsand add invalid params tests to config so they're easier to find/use the same helpers/same formatTesting
Checklist
master