feat(api): Add endpoint to allow user to resend confirmation email #543
Conversation
A new API endpoint has been added to allow a user to resend a registration confirmation to a user with the provided email. Limitations: For the MVP, this commit causes the API to rely on the Cognito service to rate limit the requests rather than keep track of the requests itself. closes #542
paulespinosa
requested review from
erikguntner,
stevbark,
tylerthome,
ju1es,
rochun and
mira-kine
| except botocore.exceptions.ClientError as error: | ||
| match error.response['Error']['Code']: | ||
| case 'UserNotFoundException': | ||
| msg = "User not found. Confirmation not sent." |
There was a problem hiding this comment.
I really like the more specific breakdown of possible errors. Might be worth doing this with the other routes
api/openapi_server/openapi/paths/auth/authResendConfirmationCode.yaml
Outdated
Show resolved
Hide resolved
|
@paulespinosa One thing to note, I checked out the branch to test the endpoint and I ran into an error with the match statements. Apparently, this feature is in versions Python 3.10+, so we might want to change the required version in the docs since it's currently at 3.5.2+ |
|
Thank you for checking. Yes, we should update the required version.
…On July 20, 2023 11:57:55 PM GMT+07:00, Erik Guntner ***@***.***> wrote:
@paulespinosa One thing to note, I checked out the branch to test the endpoint and I ran into an error with the match statements. Apparently, this feature is in versions Python 3.10+, so we might want to change the required version in the docs since it's currently at 3.5.2+
--
Reply to this email directly or view it on GitHub:
#543 (comment)
You are receiving this because you were mentioned.
Message ID: ***@***.***>
|
|
I see. I had thought it was the invitee who would make the request.
…On July 20, 2023 11:51:50 PM GMT+07:00, Erik Guntner ***@***.***> wrote:
@erikguntner commented on this pull request.
> + - email
+ responses:
+ '200':
+ description: successful operation
+ content:
+ application/json:
+ schema:
+ $ref: '../../openapi.yaml#/components/schemas/ApiResponse'
+ '400':
+ description: The email parameter was not sent or the user with the given email was not found.
+ '429':
+ description: Too many requests to resend the registration confirmation code were made to this user.
+ tags:
+ - auth
+ x-openapi-router-controller: openapi_server.controllers.auth_controller
+ security:
I don't know if we want to require an auth token for this route since users may not be signed in when making the request. For example, post-sign up, but before signing in. I think the way Cognito is currently set up, a user without a verified email won't be able to sign in
--
Reply to this email directly or view it on GitHub:
#543 (review)
You are receiving this because you authored the thread.
Message ID: ***@***.***>
|
| /auth/signin: | ||
| $ref: "./paths/auth/authSignin.yaml" | ||
| /auth/resend_confirmation_code: |
There was a problem hiding this comment.
is this suppose to be snake or camel case
There was a problem hiding this comment.
Hi @ju1es! Thank you for taking a look. This is a good and important question.
Regarding API endpoint (OpenAPI paths):
I checked again. I see there are both ways being used in this file (e.g. /serviceProviders, /auth/initialInvite, and /auth/forgot_password, /auth/forgot_password/confirm).
- When I searched for guidance in the OpenAPI documentation, I didn't see any explicit convention written, but I did see they used camelCase (this is called mixed case in Python PEP8) in their examples.
- I then checked https://restfulapi.net/resource-naming/ which only says to be consistent in whatever convention is used and provides "hints" to use hyphen (-) to separate words, avoid underscores, and use lowercase letters.
- Then, I checked the connexion documentation and saw they had an example using snake case (
/hello_world) (doh!). - Finally, I tried searching for preferences from the React side but couldn't find anything.
Do you, @erikguntner, @Joshua-Douglas know if there is a preference on the front end side regarding the API endpoint naming convention?
Regarding OpenAPI operationId:
I tend to use snake case to follow PEP8 function and variable name convention because it's implemented in Python and the auth_controller.py is dominantly snake case.
There was a problem hiding this comment.
@paulespinosa Yeah I was using snake case since that's what I saw in the Flask and connexion docs, but I defer to you all with more Python experience for what you wanna do moving forward. Camel case is the standard in Javascript land and always appreciated when getting data back from a db, but the endpoints don't necessarily need to be written in it. Hyphens look fairly clean tho
There was a problem hiding this comment.
so many options! thanks for looking into it so thoroughly @paulespinosa
+1 hyphens for me as well. i don't have a strong preference except that we adhere to one convention for consistency.
|
preemptively approving but any tests for the controller? |
I was thinking about this too. Right now, this is a wrapper around the boto3 client, and I was hesitant to mock it. I haven't read all of boto3 documentation yet. So, I'd like some help to write the tests. |
yeah I wasn't sure how to test the controllers which is why I pulled some of the logic out into the 'repositories' class. I think unit tests would be nice. thinking a little more about it, I'm leaning as long as it's been manually tested we can roll with it for now just so we can get things up and running. |
A new API endpoint has been added to allow a user to resend a registration confirmation to a user with the provided email.
Limitations:
For the MVP, this commit causes the API to rely on the Cognito service to rate limit the requests rather than keep track of the requests itself.
closes #542