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

Add oauth sign-in to registration page #26951

Closed
wants to merge 3 commits into from

Conversation

jackHay22
Copy link
Contributor

This is a follow-on to #26550 and includes oauth signin on the registration page.

This is particularly important for invitations by email. A user without an account may be invited to join a team and will receive an activation link that first navigates to the registration page. However, this will be a confusing flow if they intend to use an external auth provider.

Screen Shot 2023-09-06 at 12 55 49 PM

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Sep 6, 2023
@pull-request-size pull-request-size bot added the size/M Denotes a PR that changes 30-99 lines, ignoring generated files. label Sep 6, 2023
Copy link
Contributor

@kdumontnu kdumontnu left a comment

Choose a reason for hiding this comment

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

If users navigate first to the registration, they won't necessarily know what external auth sources are available. Most web apps I've seen have external auth sources available on the registration page.

Imo, this is only loosely related to the team invite PR.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Sep 6, 2023
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Sep 7, 2023
@lunny lunny added the topic/ui-interaction Change the process how users use Gitea instead of the visual appearance label Sep 7, 2023
@lunny lunny added this to the 1.21.0 milestone Sep 7, 2023
@lunny lunny added the reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. label Sep 7, 2023
@wxiaoguang
Copy link
Contributor

wxiaoguang commented Sep 7, 2023

Resolve

But I think it's better to move the code into a shared template. I guess the code was just copied&pasted.

And I think #20242 should be fixed and merged, because it came early but nobody reviews (well, community driven, so it's also fine to close that one and take this one)

@wxiaoguang wxiaoguang added issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail and removed reviewed/wait-merge This pull request is part of the merge queue. It will be merged soon. labels Sep 7, 2023
@lunny
Copy link
Member

lunny commented Sep 7, 2023

Resolve

* [External openid connect (oidc, oauth) providers missing on sign up/register page #19809](https://github.com/go-gitea/gitea/issues/19809)

* [Show OpenID Connect and OAuth on signup page #20242](https://github.com/go-gitea/gitea/pull/20242)

But I think it's better to move the code into a shared template. I guess the code was just copied&pasted.

And I think #20242 should be fixed and merged, because it came early but nobody reviews (well, community driven, so it's also fine to close that one and take this one)

So let's review that one. I think that PR includes all of this PR.

@jackHay22
Copy link
Contributor Author

Resolve

* [External openid connect (oidc, oauth) providers missing on sign up/register page #19809](https://github.com/go-gitea/gitea/issues/19809)

* [Show OpenID Connect and OAuth on signup page #20242](https://github.com/go-gitea/gitea/pull/20242)

But I think it's better to move the code into a shared template. I guess the code was just copied&pasted.
And I think #20242 should be fixed and merged, because it came early but nobody reviews (well, community driven, so it's also fine to close that one and take this one)

So let's review that one. I think that PR includes all of this PR.

Agreed, that PR looks nearly identical to mine 👍

@jackHay22
Copy link
Contributor Author

Replaced by: #20242

@jackHay22 jackHay22 closed this Sep 12, 2023
@GiteaBot GiteaBot removed this from the 1.21.0 milestone Sep 12, 2023
@go-gitea go-gitea locked as resolved and limited conversation to collaborators Dec 11, 2023
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/needs-feedback For bugs, we need more details. For features, the feature must be described in more detail lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. size/M Denotes a PR that changes 30-99 lines, ignoring generated files. topic/ui-interaction Change the process how users use Gitea instead of the visual appearance
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

5 participants