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

Enable use of secure resources #53 #54

Merged
merged 2 commits into from Oct 24, 2022
Merged

Conversation

syedfkabir
Copy link
Contributor

@syedfkabir syedfkabir commented Oct 24, 2022

Resolves #53

Changes

  • added <RequireLogin /> component to <KettingProvider />
  • added a small onSuccess helper

Notes

Steps so far…

Made sure KettingProvider is encapsulating RequireLogin

Made sure RequireLogin encapsulates Routes

Could not use config found in storysuite
so I hard coded the token and authorizeEndpoints param
This wasn’t so difficult as the config just produced the a12nserver’s uri

Added a onSuccess that redirects the user to the home page ( ‘/‘ )

When hard coding the cliendId, I realized I needed to create the app in a12n get the clientId
Made tt-app in a12n
Made sure to add http://localhost:8902/ as a valid redirectUri (otherwise would not work by default)

The front end would give a ‘Not Authorized’ page when running an ‘authenticated backend’.
After implementing the steps above, I am no longer getting a ‘Not Authorized’ page
Instead I am getting the home page, or whatever page I was on.

this works, but I expected there to be a login process for the FE as well,
Just as there is one for the BE currently.
If I understand correctly, I may have reached the end of what I can do for this PR.
Currently there is no ‘access control’ in the TimeTracker,
I’m assuming I need to implement that before I get the login screen as expected on the FE.

@syedfkabir syedfkabir changed the title Implement Authentication #53 Enable use of secure backend resources #53 Oct 24, 2022
@syedfkabir syedfkabir changed the title Enable use of secure backend resources #53 Enable use of secure resources #53 Oct 24, 2022
Copy link
Collaborator

@evert evert left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This all looks pretty good to me. I assume you tested this, and it works?

In a follow-up ticket we should move the a12n url and the tt-api urls into the environment, but that doesn't have to happen here

@evert
Copy link
Collaborator

evert commented Oct 24, 2022

2 last notes:

  1. Instead of asking users to set up the redirect uri, provide a similar link the README.md just like we did for tt-api.
  2. If you didn't get a login screen and immediately redirected back, you probably were simply already logged into a12nserver.

If 1 is resolved, this can merge! Good to have the tt-app merged before tt-api because it will still function even if the tt-api stuff isn't there yet.

@syedfkabir
Copy link
Contributor Author

  1. Instead of asking users to set up the redirect uri, provide a similar link the README.md just like we did for tt-api.

DONE!!

  1. If you didn't get a login screen and immediately redirected back, you probably were simply already logged into a12nserver.

Yes! And if I wasn't logged into a12n or tt-api, I'd get a 'not authenticated' screen as expected.

@syedfkabir syedfkabir merged commit ec0aaf3 into main Oct 24, 2022
@syedfkabir syedfkabir deleted the sk/53/implement-authentication branch October 24, 2022 17:50
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

Successfully merging this pull request may close these issues.

Enable use of secure resources
2 participants