-
Notifications
You must be signed in to change notification settings - Fork 484
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
Fix a bug where JWT on relative paths #604
Conversation
Any prediction when this is going to be merged & released ? |
Are you aware this a duplicate of pull request #542 ? |
How is this a duplicate of #542 ? |
Sorry, commented on wrong issue. |
Was meant for PR #600 |
src/jwt.interceptor.ts
Outdated
@@ -40,15 +40,15 @@ export class JwtInterceptor implements HttpInterceptor { | |||
|
|||
isWhitelistedDomain(request: HttpRequest<any>): boolean { | |||
const requestUrl: any = parse(request.url, false, true); | |||
|
|||
const requestHost: string = requestUrl.host || (location && location.host) |
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.
In order to not block potential universal support, can we do a typeof
check here on location?
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.
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.
Done
Reverting this merge as it introduces an undesirable change to the default whitelisting behaviour. See 0287bcb#r34107408 This reverts commit 33e441f, reversing changes made to 5f8db7f.
Revert "Merge pull request #604 from driverpt/patch-1"
@stevehobbsdev , that is the correct behaviour. If you're redirecting to an S3 Bucket, since the initial request to the server has no Host, it will always add the JWT Headers, which will result in 403 Errors from AWS. |
Fixes #603