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

Add support for custom cognito redirect path #87

Merged
merged 3 commits into from
Mar 1, 2024

Conversation

fknittel
Copy link
Contributor

Description of changes:
We are using cognito-at-edge in an environment where not all paths are handled by our edge lambda. Specifically, the website root / is not handled by us.

Example:

An access attempt to https://example.com/foo/ results in a redirect to https://domain.auth.us-east-1.amazoncognito.com/authorize?redirect_uri=https://example.com&response_type=code&client_id=63gcbm2jmskokurt5ku9fhejc6&state=/foo/

Notice that the redirect_uri points to a path that is not handled by us.

This PR adds an option redirectPath to allow specifying a different value for redirect_uri. Example:

const authenticator = new Authenticator({
  region: 'us-east-1',
  userPoolId: 'us-east-1_tyo1a1FHH',
  userPoolAppId: '63gcbm2jmskokurt5ku9fhejc6',
  userPoolDomain: 'domain.auth.us-east-1.amazoncognito.com',
  redirectPath: '/foo/oauth2/idresponse',
});

An access attempt to https://example.com/foo/ now results in a redirect to https://domain.auth.us-east-1.amazoncognito.com/authorize?redirect_uri=https://example.com/foo/oauth2/idresponse&response_type=code&client_id=63gcbm2jmskokurt5ku9fhejc6&state=/foo/

I hope the use-case makes sense to you and isn't too much of an edge case. (Otherwise I'd love to learn of a way to get the same result without modifying or effectively forking cognito-at-edge.) I'd be more than happy to tweak the naming, add more automated tests, etc.

The modified code appears to work well and is already used in production.

By submitting this pull request, I confirm that my contribution is made under the terms of the Apache 2.0 license.

In case not all paths are handled by the cognito-at-edge, allow to
specify a path to which cognito will redirect via the new `redirectPath`
option.  The default remains the website root.
@gabrielcho
Copy link

gabrielcho commented Jan 26, 2024

It would be great to have this implemented.
Answering your request of finding a way to get the same result without forking: I was experimenting with the parseAuthPath field used in the Authenticator constructor object parameter and I managed to get a redirect_uri that includes it on the first unauthenticated call cognito-at-edge does to the cognito api /login. The problem comes when you are calling cognito-at-edge again, (after successful login) with the generated code you got from the previous call, the redirect_uri doesn't have the parseAuthPath parameter on the request made by lambda@edge to the cognito endpoint /oauth2/token to actually validate that code.

@jeandek jeandek self-assigned this Feb 28, 2024
@jeandek jeandek added the added-feature For PRs which containg a new feature (may be in response to a `feature-request`) label Feb 28, 2024
@jeandek jeandek added this to the 1.5.1 milestone Feb 28, 2024
@jeandek
Copy link
Contributor

jeandek commented Feb 28, 2024

@fknittel Hi Fabian, sorry for the lack of response from the team on this PR. Unfortunately, we have a lot of conflicting priorities which take precedence over maintaining and extending this package.

It turns out that we have a very similar need to yours, though, so I will be happy to review your CR and merge it ASAP.

There already was a `parseAuthPath` parameter which `redirectPath` would be potentially conflict with. Reusing `parseAuthPath` maintains compatibility with previous versions.
@jeandek jeandek merged commit 2ddc133 into awslabs:main Mar 1, 2024
1 check passed
@jeandek
Copy link
Contributor

jeandek commented Mar 1, 2024

I needed to maintain compatibility with the parseAuthPath parameter which already existed, so I made a few changes. Thanks for your contribution! Expect a new release early next week.

@jeandek jeandek added bug Something isn't working and removed added-feature For PRs which containg a new feature (may be in response to a `feature-request`) labels Mar 1, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants