Location via proxy:   [ UP ]  
[Report a bug]   [Manage cookies]                
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

Feedback Items For Consideration #1

Open
kinowarrior opened this issue May 31, 2019 · 3 comments
Open

Feedback Items For Consideration #1

kinowarrior opened this issue May 31, 2019 · 3 comments

Comments

@kinowarrior
Copy link

kinowarrior commented May 31, 2019

@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 :)

React_App

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).

incorrect-secret

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 inside aws-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 for REACT_APP_OAUTH_DOMAIN, REACT_APP_API_HOST and REACT_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.

@kinowarrior kinowarrior changed the title aws-api/package.json issue with update-api Feedback Items For Consideration May 31, 2019
@csepulv
Copy link
Owner

csepulv commented Jun 1, 2019

These are good suggestions. I will try to find some time to make the tweaks.

@kinowarrior
Copy link
Author

kinowarrior commented Jun 1, 2019

@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 aws-api, and migrate the contents of the scripts folder over to terraform. We could define ALL the IAM roles using terraform which would reduce the need to add any explicit inline policy step for the authenticated user situation. Serverless framework offers the ability to reference the ARN of the role (as well as the ability to define it and reference at same time within the yml config. Same as Claudia.js (which I believe you mentioned too).

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.

@csepulv
Copy link
Owner

csepulv commented Jun 2, 2019

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 aws-api directory. Did you run from different directories?

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)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants