-
Notifications
You must be signed in to change notification settings - Fork 38
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
Feedback Items For Consideration #1
Comments
update-api
These are good suggestions. I will try to find some time to make the tweaks. |
@csepulv - I could help out with a PR for 1) and 3) above if that helps. I'd also like to port from claudia.js to the serverless framework the contents of Would you be able to explain very briefly the touch points in the sample to refactor towards a social connection (OIDC/OAuth2) with say Facebook or Google. I'd like to create a separate branch with that connection type - still using Amplify on the Client (React) side. |
I've posted an update for the (1) and (3). For (2), I think it works as is. You need to run the claudia commands from the On serverless and terraform, I prefer to not make those type of changes. It would require changes to the tutorial. The sample code is meant as an example, not a boilerplate. If you would like to create a fork with those changes, I would be happy to link to it from the main README. On your last comment, on social logins, are you looking to add them via Cognito or independent of Cognito. If they are configured on the Cognito side, you don't need to change anything on the client/react side. (I use this setup and it is only a AWS side config change) |
@csepulv - Thanks so much for the comprehensive tutorial and source code. You did a great job explaining everything, and I enjoyed following along. Did eventually successfully get the example running locally too :)
There were only four points wished to offer in the interests of sharing my experience:
1). In the
aws-setup.md
, there is confusion about needing to tick the Client Secret option when setting up the Cognito User Pool Client.You show a screenshot with the
Generate Client Secret
ticked. This causes the/token
endpoint to break later on. If you do generate a client secret you must pass it with the client id on that endpoint (which the web app is not configured to do).2). The
aws-api
package.json script for creation of lambda/api gateway works well, however the update-api script is broken. It seems to expect the package.json file to reside at the base of the project (ie. one directory level higher than where it is now. I wonder whether when you bundled it insideaws-api
folder, it inadvertently broke it?3). Would have welcomed a README inside the
web-ui
directory (not the auto-generated create-react-app one) explaining each setting that the.env
file would need. For instance, some settings were really clear, but I had less certainty where to get the value forREACT_APP_OAUTH_DOMAIN
,REACT_APP_API_HOST
andREACT_APP_API_KEY
. It's like anything, the extra work actually forced me to think through the issues and I got a better learning experience as a result. But, if you're looking for a smooth tutorial with no sweat, then that would add value.4). Would have really welcomed some prose on the auth flow itself. We just ticked accept implicit grant and authorization grant flows, opted to use hosted UI and that was it. Given we have a SPA app (using React.js), but we're using a Hosted UI for the Identity Provider piece, it was less clear precisely what your overall upfront design was. Plus Cognito adds its own magic with tokens, and credentials etc so more information here would have been helpful. Make sense?
Again, thanks so much. Great Cognito samples with solid documentation seem to be in short supply and I found this a great start.
The text was updated successfully, but these errors were encountered: