This repository has been archived by the owner on Mar 22, 2022. It is now read-only.
Don’t mix options when signing tokens #255
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
| noTimestamp, | ||
| header | ||
| }; | ||
|
|
||
| if (!data.iss) { |
There was a problem hiding this comment.
Confused a bit about this one @marshallswain. Maybe I'm missing something. Any reason you want to pass those as data?
I was thinking any token options should come from params.jwt and due to the object deconstruction above this, those fields will just be undefined if they weren't passed along via params.jwt or aren't a default value.
There was a problem hiding this comment.
They are already getting passed when you login with a token, which is
causing an error.
On Wed, Aug 3, 2016 at 8:37 PM Eric Kryski notifications@github.com wrote:
In src/services/token.js
#255 (comment)
:noTimestamp, header };
- if (!data.iss) {
Confused a bit about this one @marshallswain
https://github.com/marshallswain. Maybe I'm missing something. Any
reason you want to pass those as data?I was thinking any token options should come from params.jwt and due to
the object deconstruction above this, those fields will just be undefined
if they weren't passed along via params.jwt or aren't a default value.—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
https://github.com/feathersjs/feathers-authentication/pull/255/files/0edd5ccefaad58a09eed1c58d9b8f1c1f23ba6e8#r73454675,
or mute the thread
https://github.com/notifications/unsubscribe-auth/AAH3WX6lz5v2EhvQ22Kq6UcSkcWEmFB8ks5qcVBZgaJpZM4JZKVM
.
ekryski
added a commit
that referenced
this pull request
Nov 16, 2016
* cleaning up dependencies * removing auth redirects and exposing middleware * expanding express middleware * moving what I can in services to setup method * updating dependencies * cleaning up middleware and adding debug logs * cleaning up services and adding debug logs * changing options for populate user hook to conform with other options * cleaning up main index file * fixing lint errors * getting example app working * fixing options for populate user middlware * fixing socket logout emitting * restructuring so you can set hooks to construct your token payload if you want to customize it * Default to a session cookie instead of 1 day * Switch to "user" instead of "data" for the response from auth * Make sure we clear the user out of locals so that you don't end up in a weird state. * Allow passing options when creating a JWT. Used to customize the type of token we want to generate (ie. confirmation, password reset, etc.) * setting version * don't throw an error in the decode token middleware * bump version * clearing cookie if use not found. Setting cookie age to same as JWT * bump version * Don’t mix options when signing tokens (#255) Fixes #254. * Attempt to get token right away. (#252) * Attempt to get token right away. This makes it so that we don’t have to wait for an async response in order to start making authenticated requests. * Also set up localStorage. * fix restrict to owner hook methods. Closes #228 * bump version * cookies should get set regardless of whether it was an xhr request if enabled * bumping version * adding migration guide * reorganizing middleware, hooks and services * updaing mocha * updating migration doc of things left to doc/complete * fixing a bunch of the tests and adding tests for all new middleware * cleaning up client side tests. Still failing * getting all tests passing * updating all middleware to not have default and pull from global config * finished consolidating options * bumping version * adding more details to migration guide * fix typo missing 'd' * Fix typo and simplify wording. * normalize the callbackURL If the callbackURL doesn’t begin with a slash, the Passport `authenticate()` call will mess up the URL depending on the referrer’s url. So if the page is `domain.com/auth/github`, the callback URL will become `domain.com/auth/auth/github`. This normalizes the URL so that relative URLs always begin with a leading slash and absolute URLs don’t get touched. * Make sure the provider plugin name doesn't overwrite the OAuth provider name * consistency: `callbackUrl` should be `callbackURL` * Normalize comparison URL & fix typo This performs the same normalization to the comparison URL for the callback (as is done for the callbackURL). Also fixes typo in callbackUrl, updates it to callbackURL. * Always use service lookup. Previously, we were setting up this._tokenService and this._userService in the `setup` function. This was a problem because not all services are registered by the time `setup` runs. In order to continue to allow the passing of either strings or an actual service object, this PR checks if we only have a string reference to the service stored. If so, it uses the `app.service` method of service lookup before attempting to use the service. This also fixes a problem that was occurring when trying to call this._userService or this._tokenService when this was undefined inside the middleware callbacks. * DRY up the dynamic token and user service lookup. This moves the dynamic service lookup to a function to be a bit more DRY. * OAuth require successRedirect with default successHandler If a successHandler hasn’t been passed, then the default `successHandler` will be used. The `successRedirect` will now be required in that scenario. * First cut for authentication middleware (#305) * First cut for authentication middleware * Fix service methods * Allow passing user service and service name * First cut for authentication middleware (#305) * First cut for authentication middleware * Fix service methods * Allow passing user service and service name * Cookies will match jwt expiry by default. (#308) * Cookies will match jwt expiry by default. * Add missing makeExpiry function. * Store config at `app.config` (#312) * Store config at `app.config` * Use app.set to store config. * Add test. * adding instanbul code coverage * Remove permissions hooks and middleware which will be put into feathers-permissions (#307) * Started implementation of more modularized module structure * Some reorganization * Implement Socket new authentication * More reorganization and start of integration tests * eslint fix * More integration tests and cleanup * reogranizing * Applying latest changes and merging with dev other branch * Socket.io authentication tests and login logout event * Improving socket tests and adding Primus * Some cleanup * Better error verification tests * Implement login and logout events for REST authentication (#325) * Fix tests * wip * first cut of auth working with passport. Clean up and tests to do * fixing event middleware resolution * Keep github together * Keep twitter together * getting tests passing. Still a couple more to do * removing unused hooks for now. May bring some back later * fixing lint errors * removing hashPassword hook. It now lives in feathers-authentication-local * adding a migration guide and new features docs * adding more detail to migration doc * cleaning up dependencies * getting tests passing again * adding some more tests. Implementing chained strategies * cleaning up dependencies * finishing integration tests and handling socket logout timeout * cleaning up example app * fixing up example * updating README * updating API docs
Sign up for free
to subscribe to this conversation on GitHub.
Already have an account?
Sign in.
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.
Fixes #254. Allows the attributes on
datato override the options passed tojwt.sign.