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

Merge seperate config schemas #57

Merged
merged 8 commits into from
Jan 27, 2020
Merged

Merge seperate config schemas #57

merged 8 commits into from
Jan 27, 2020

Conversation

joshcanhelp
Copy link
Contributor

@joshcanhelp joshcanhelp commented Jan 23, 2020

Description

  • 268a907 - Merge appSessionCookie schema into main config schema to remove double-processing
  • 5b274f4 - Merge authorizationParams schema into main config schema to remove double-processing
  • 44894ce - Default to response_type=form_post if response_mode contains token or id_token; change default postLogoutRedirectUri to not append trailing slash
  • 7deb960 - Allow response_type to respect content rather than exact match against issuer metadata
  • c88a5d4 - Move loadEnv functionality into config.js
  • a709b2d - Move clientSecret required checks into Joi when() statements

References

assert.equal(parsed.query.redirect_uri, 'https://example.org/callback');
assert.property(parsed.query, 'nonce');
assert.property(parsed.query, 'state');
});

it('should contain the two callbacks route', function() {
assert.ok(router.stack.some(filterRoute('GET', '/callback')));
assert.ok(router.stack.some(filterRoute('POST', '/callback')));
Copy link
Contributor Author

Choose a reason for hiding this comment

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

This was a remnant of fragment support. id_token with an undefined response_mode defaults to fragment which is not supported in this SDK.

@@ -104,7 +104,7 @@ describe('invalid parameters', function() {
httpOnly: '__invalid_httponly__'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Error message changes only in this file.

@@ -44,7 +43,7 @@ describe('logout route', function() {

it('should redirect to the base url', function() {
assert.equal(logoutResponse.statusCode, 302);
assert.equal(logoutResponse.headers.location, 'https://example.org/');
assert.equal(logoutResponse.headers.location, 'https://example.org');
Copy link
Contributor Author

Choose a reason for hiding this comment

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

const config = getConfig(customConfig);

it('should keep token code', function() {
assert.equal(config.authorizationParams.response_type, 'token id_token code');
Copy link
Contributor Author

@joshcanhelp joshcanhelp Jan 23, 2020

Choose a reason for hiding this comment

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

Does not exactly match metadata fixture so this would have thrown before.

@joshcanhelp joshcanhelp added this to the v0.7.0 milestone Jan 24, 2020
@joshcanhelp joshcanhelp marked this pull request as ready for review January 24, 2020 23:01
@joshcanhelp joshcanhelp requested a review from a team January 24, 2020 23:01
Copy link

@stevehobbsdev stevehobbsdev left a comment

Choose a reason for hiding this comment

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

LGTM, just left a minor thing.

test/config.tests.js Outdated Show resolved Hide resolved
Co-Authored-By: Steve Hobbs <steve.hobbs.mail@gmail.com>
@joshcanhelp joshcanhelp merged commit 46c27d2 into auth0:master Jan 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants