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

feat(story): add authentication #9

Merged
merged 3 commits into from
Nov 8, 2023

Conversation

eunjae-lee
Copy link
Contributor

@eunjae-lee eunjae-lee commented Oct 13, 2023

What?

This PR adds oauth flow.

Find more explanation at https://storyblok.slack.com/archives/C04P3JBHWHW/p1697208267013089

Why?

JIRA: EXT-1994

How to test? (optional)

@eunjae-lee
Copy link
Contributor Author

eunjae-lee commented Oct 13, 2023

Current dependencies on/for this PR:

This comment was auto-generated by Graphite.

@eunjae-lee eunjae-lee force-pushed the EXT-1994-add-authentication-to-nuxt-starter branch 3 times, most recently from 1e495ea to e099f47 Compare October 13, 2023 15:15
@eunjae-lee eunjae-lee marked this pull request as ready for review October 13, 2023 15:17
@eunjae-lee eunjae-lee force-pushed the EXT-1994-add-authentication-to-nuxt-starter branch 2 times, most recently from fce969d to fb5ab32 Compare October 13, 2023 15:21
Copy link
Contributor

@BibiSebi BibiSebi left a comment

Choose a reason for hiding this comment

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

👏

Copy link
Contributor

Choose a reason for hiding this comment

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

This middleware approach is really cool and better, compared with what we had before.

@eunjae-lee, is there any chance of this one in particular not being reachable because the story-starter/starters/nuxt/server/middleware/auth.ts would take precedence over this one?

I'm wondering if the auth is going to be executed first and if so, may this one will never be reachable.

Do you think there is this chance?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@demetriusfeijoo good point!
I've renamed them according to this rule

Copy link
Contributor

@demetriusfeijoo demetriusfeijoo left a comment

Choose a reason for hiding this comment

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

Thanks for your effort in this one, @eunjae-lee. 🚀

@eunjae-lee
Copy link
Contributor Author

@BibiSebi @demetriusfeijoo by the way, I'm not going to merge this PR and its parent PR. We're going to stack more PRs on top of this, and try to merge them altogether via Graphite. So, don't merge this by accident! :)

@eunjae-lee
Copy link
Contributor Author

eunjae-lee commented Nov 8, 2023

Merge activity

  • Nov 8, 10:06 AM: @eunjae-lee started a stack merge that includes this pull request via Graphite.
  • Nov 8, 10:06 AM: Graphite rebased this pull request as part of a merge.
  • Nov 8, 10:07 AM: @eunjae-lee merged this pull request with Graphite.

Base automatically changed from chore/clean-up to main November 8, 2023 15:06
@eunjae-lee eunjae-lee force-pushed the EXT-1994-add-authentication-to-nuxt-starter branch from 48e90f7 to 7c03b6d Compare November 8, 2023 15:06
@eunjae-lee eunjae-lee merged commit 16e1614 into main Nov 8, 2023
1 check passed
@eunjae-lee eunjae-lee deleted the EXT-1994-add-authentication-to-nuxt-starter branch November 8, 2023 15:07
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.

3 participants