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

DirectAuth updates for Storage #1248

Merged
merged 21 commits into from
Jul 9, 2021
Merged

DirectAuth updates for Storage #1248

merged 21 commits into from
Jul 9, 2021

Conversation

RyRy79261
Copy link
Contributor

closes #1207

@RyRy79261 RyRy79261 added Epic: Storage v0 Type: Feature Added to PRs to identify that the change is a new feature. labels Jul 7, 2021
@RyRy79261 RyRy79261 self-assigned this Jul 7, 2021
@render
Copy link

render bot commented Jul 7, 2021

@render
Copy link

render bot commented Jul 7, 2021

@RyRy79261 RyRy79261 marked this pull request as ready for review July 8, 2021 13:52
@RyRy79261 RyRy79261 requested review from tanmoyAtb, Tbaut and FSM1 and removed request for tanmoyAtb and Tbaut July 8, 2021 13:52
@RyRy79261 RyRy79261 added the Status: Review Needed 👀 Added to PRs when they need more review label Jul 8, 2021
@RyRy79261 RyRy79261 requested review from Tbaut and asnaith July 9, 2021 08:51
Copy link
Collaborator

@Tbaut Tbaut left a comment

Choose a reason for hiding this comment

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

Working super well. Found a couple nits but that's it :)

packages/storage-ui/src/Contexts/StorageApiContext.tsx Outdated Show resolved Hide resolved
packages/storage-ui/src/Contexts/StorageApiContext.tsx Outdated Show resolved Hide resolved
const getIdentityToken = async (
loginType: IdentityProvider,
tokenInfo?: {token: IdentityToken; email: string}
): Promise<{identityToken: IdentityToken; userInfo: any}> => {
Copy link
Collaborator

Choose a reason for hiding this comment

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

Any way we can type this strongly?

Copy link
Contributor

Choose a reason for hiding this comment

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

I played around with it and have not managed to come up with a type definition that works. Given the tiny surface area of this type, I would say we should leave it as is.

@FSM1 FSM1 enabled auto-merge (squash) July 9, 2021 11:18
@FSM1 FSM1 merged commit 32ec2c6 into dev Jul 9, 2021
@FSM1 FSM1 deleted the feat/direct-auth-1207 branch July 9, 2021 11:19
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Status: Review Needed 👀 Added to PRs when they need more review Type: Feature Added to PRs to identify that the change is a new feature.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Update OAuth and add email login for Storage
4 participants