-
Notifications
You must be signed in to change notification settings - Fork 5
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
Token Validation for API calls, fix #354 #372
Conversation
The latest updates on your projects. Learn more about Vercel for Git ↗︎
|
@kamicut does this close issue 354? |
Yes! I forgot the issue number, it fixes #354 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kamicut this look good overall, lets try to add test coverage for a HTTP agent, as discussed.
* purposes, it mocks the hydra access token introspection | ||
*/ | ||
export default async function handler(req, res) { | ||
if (req.method === 'POST') { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kamicut I suggest adding a check here to avoid exposing this route outside testing:
if (process.env.TESTING !== 'true' || req.method !== 'POST') {
throw Boom.notFound()
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@kamicut I think we are almost there, should we add a test for a route in 'manage' folder?
This implements:
Bearer <access_token>
/introspect
route in the auth proxy (check 5470dc5)sub
trait from the resulting token and adds it to the sessionThis will allow API calls from third party applications to use access tokens to authenticate with the API.
I also added
isAuthenticated
for routes that need simply to have the session and session user ids in thecan
middleware.