Skip to content
This repository was archived by the owner on Feb 8, 2024. It is now read-only.

Change the application access authentication flow#1278

Merged
ryanclark merged 11 commits intomasterfrom
ryan/change-app-access-login-flow
Jan 12, 2023
Merged

Change the application access authentication flow#1278
ryanclark merged 11 commits intomasterfrom
ryan/change-app-access-login-flow

Conversation

@ryanclark
Copy link
Copy Markdown
Member

This is the frontend changes for this PR - gravitational/teleport#17592

This sets a custom X-Cookie-Value header on the request to /x-teleport-auth instead of putting it in the POST body.

We also remove a lot of the redirects (and the code that goes with it) as it's no longer required with the new authentication flow in the Teleport PR.

@ryanclark ryanclark marked this pull request as draft October 19, 2022 18:55
@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch from feb4339 to 51f4eb9 Compare November 23, 2022 17:58
@ikkisoft
Copy link
Copy Markdown

LGMT. Comments left in #17592

@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch from 94aec56 to 56ebf74 Compare November 30, 2022 20:31
@ryanclark ryanclark marked this pull request as ready for review November 30, 2022 20:31
@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch from 56ebf74 to de7d7bf Compare December 7, 2022 16:44
@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch from de7d7bf to c3400ac Compare December 15, 2022 00:11
@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch from c3400ac to bedbf55 Compare December 23, 2022 16:56
@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch 3 times, most recently from d33be45 to 7618156 Compare January 4, 2023 15:56
@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch from 7618156 to 4345956 Compare January 12, 2023 17:12
Copy link
Copy Markdown
Contributor

@kimlisa kimlisa left a comment

Choose a reason for hiding this comment

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

We also remove a lot of the redirects (and the code that goes with it) as it's no longer required with the new authentication flow in the Teleport PR.

that means like url params like awsrole and state right? can we also delete useAppLaunder file since it doesn't look like it's being used. also does the backend PR delete the code that does the redirecting? eg redirectToLauncher, before I saw the above comment, i thought we missed these params

Comment thread packages/teleport/src/AppLauncher/AppLauncher.tsx Outdated
Comment thread packages/teleport/src/AppLauncher/AppLauncher.tsx
@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch from 03075f5 to 2e86b89 Compare January 12, 2023 20:09
@ryanclark
Copy link
Copy Markdown
Member Author

ryanclark commented Jan 12, 2023

that means like url params like awsrole and state right? can we also delete useAppLaunder file since it doesn't look like it's being used. also does the backend PR delete the code that does the redirecting? eg redirectToLauncher, before I saw the above comment, i thought we missed these params

state is no longer needed - the backend changes remove the ?state= dance

arn is preserved and passed through to createAppSession if it's present (it was put into ?awsrole= during the redirects but we no longer need that)

image

I forgot about ?path= and have updated the PR to redirect to the given path if it's in the query params - nice catch!

I've also removed useAppLauncher

@ryanclark ryanclark force-pushed the ryan/change-app-access-login-flow branch from be61145 to 0e20433 Compare January 12, 2023 20:17
@ryanclark ryanclark enabled auto-merge (squash) January 12, 2023 20:55
@ryanclark ryanclark merged commit f2dc620 into master Jan 12, 2023
@ryanclark ryanclark deleted the ryan/change-app-access-login-flow branch January 12, 2023 21:25
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants