140 cognito#168
Conversation
…added required env keys to example.env, configured amplify in separate auth file only if both environment variables exist/are set.
…igure runs correctly with cognito values as strings.
…array of strings or a single string.
…e (COGNITO_USER_POOL_ID is set), but missing env variables for client_id and cognito_region both should not allow requests to go through
…into 140-Cognito
…into 140-Cognito Note: added necessary packages for aws-amplify. Did not push merged changes that existed inside .nx
…into 140-Cognito
dburkhart07
left a comment
There was a problem hiding this comment.
Amazing! Very educational for me to read through as well!
…or greater accessiblity
… of never type for mock requests
… any env variables)
…autogenerated in vite.config, also set the project root to allow frontend access to root /.env file.
…into 140-Cognito
|
Fell down an authentication/authorization rabbit hole atm will update with new changes soon for a better approach for an authentication-only module that still allows authorization (RBAC) later down the line |
CognitoJWTPayload to just AccessTokenPayload for accuracy, javadoc comments
…into 140-Cognito
| # Note: Leaving any field unset disables auth ENTIRELY | ||
| COGNITO_USER_POOL_ID=us-east-2_AbCdEf123 | ||
| COGNITO_CLIENT_ID=4h57k9lmno1pqrstuv2wxyz3ab | ||
| COGNITO_REGION=us-east-2 |
There was a problem hiding this comment.
we really shouldnt specify a specific COGNITO_REGION. This should just be one single AWS_REGION variable the project uses all AWS things for
There was a problem hiding this comment.
should I just leave the other fields empty as well?? I'll update the comment to say "empty or missing" instead of "unset" as well
| const request = context.switchToHttp().getRequest<Request>(); | ||
| const token = extractBearerToken(request); | ||
| if (!token) { | ||
| throw new UnauthorizedException(); |
There was a problem hiding this comment.
Can we give this exception (and the ones below) specific exception messages as well?
There was a problem hiding this comment.
I added a specific message to this one specific exception regarding a missing bearer token in a commit, but imo i think it'd be better to leave the exceptions that are thrown when validating the jwt to be general unauthorized exceptions that dont reveal internal information.
There was a problem hiding this comment.
can log the exception information instead actually
| */ | ||
| async canActivate(context: ExecutionContext): Promise<boolean> { | ||
| // If authentication is not enabled, allow the request to proceed | ||
| if (!isAuthEnabled()) { |
There was a problem hiding this comment.
Can we add some form of log here so that the user knows they are bypassing it perhaps?
| getUser(request: Request): AccessTokenPayload | null { | ||
| // If authentication is not enabled, return null | ||
| if (!isAuthEnabled()) { | ||
| return null; |
There was a problem hiding this comment.
can we maybe add some logs into these return nulls (and if there is anywhere else this is happening in the module)? that way the user knows whether the issue is cognito being down, or their user actually being unreachable.
| typeof payload.iss === 'string' && | ||
| typeof payload.token_use === 'string' && | ||
| typeof payload.exp === 'number' && | ||
| typeof payload.iat === 'number' |
There was a problem hiding this comment.
should check that this is in the past
There was a problem hiding this comment.
hmm, not sure what you mean by the past? jwt.verify will check for expired tokens though and return err on the guard.
| * @param value - The value to check, typically a decoded JWT payload. | ||
| * @returns `true` if `value` matches the {@link AccessTokenPayload} shape. | ||
| */ | ||
| export function isAccessTokenPayload( |
There was a problem hiding this comment.
do we have special checks for the client_id? what about cognito_groups
| client_id?: string; // Client ID used during authentication (ex: <your client ID>) | ||
| exp: number; // Expiration time (Unix timestamp) | ||
| iat: number; // Issued at time (Unix timestamp) | ||
| email?: string; // Email (ex: test@example.com) |
There was a problem hiding this comment.
what do we need this field for? i dont think access tokens normally send email addresses through that. i think the verify in jwt.strategy doesnt even need the email.
ℹ️ Issue
Closes #140
📝 Description
Created an easily-importable NestJS guard that can be used by future projects to implement Cognito Authentication into their application. Amplify on the frontend authenticates users, providing registered users with a JWT access token that can be used to access authorized routes.
Briefly list the changes made to the code:
✔️ Verification
Backend TESTS:


Frontend TESTS:


No Auth:
With Auth:
🏕️ (Optional) Future Work / Notes
@aws-amplify/ui-react/styles.csslibrary on main.tsx to use their premade styling?NEW: I think that a workshop on Authentication / Authorization would be helpful for future devs, would 100% be up to helping with that! I spent wayyy too much time figuring out the difference between the two and their uses in larger apps like which scaffolding will fork off of.