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

[WIP] OpenId login support #271

Closed
wants to merge 1 commit into from
Closed

Conversation

strk
Copy link
Member

@strk strk commented Nov 26, 2016

Far from ready, but open for contribution (mostly needs
UI and UX)

@andreynering andreynering added this to the 1.1.0 milestone Nov 26, 2016
@andreynering andreynering added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Nov 26, 2016
@lunny lunny added the type/proposal The new feature has not been accepted yet but needs to be discussed first. label Nov 27, 2016
@stevenroose
Copy link

Fixes #185

@tboerger tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Nov 27, 2016
@strk strk force-pushed the openid-gitea branch 4 times, most recently from 1e51a25 to 8c4784d Compare December 3, 2016 11:38
@strk strk force-pushed the openid-gitea branch 3 times, most recently from 7bbaea1 to 174a949 Compare December 13, 2016 09:35
@@ -100,6 +100,19 @@ func SignIn(ctx *context.Context) {
ctx.HTML(200, tplSignIn)
}

// OpenidVerify handles response from OpenID provider
func OpenidVerify(ctx *context.Context) {
Copy link
Member

Choose a reason for hiding this comment

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

Should this be renamed to OpenIDVerify for consistency? It seems that most other names use OpenID instead of Openid?

@@ -85,6 +85,7 @@ type User struct {
Repos []*Repository `xorm:"-"`
Location string
Website string
Openid string
Copy link
Member

Choose a reason for hiding this comment

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

Should this be renamed OpenID for consistency?

@strk
Copy link
Member Author

strk commented Dec 14, 2016 via email

@stevenroose
Copy link

stevenroose commented Dec 14, 2016

@strk I don't think OpenID as an "authentication method" makes a lot of sense. (It is a valid authentication method though, so it does make sense.)

The big advantage I would see from having OpenID is as a way of users to login without having been registered. Just like systems like Discuss allow anyone with an OpenID (even though Discuss requires this to be from a small set of providers) to make contributions in the form of issues or (federated) pull requests.

A possible setup that would benefit greatly from OpenID is the case of single-user usage. Gitea as a self-hosted Git homepage in which only one user has commit access, while he can allow anyone to open issues or request pulls with their OpenID (and a CAPTCHA ofc).

@strk
Copy link
Member Author

strk commented Dec 14, 2016 via email

@allandegnan
Copy link

So actually we (https://github.com/UKHomeOffice) use OpenIDConnect to facilitate single sign on back to a single auth provider and we occasionally use SAML to provide the same thing.

Whilst I'm not saying that the addiontion of this feature itself would make us a gitea shop, not having it is a bit of a blocker when it comes to enteprisr adoption.

@strk
Copy link
Member Author

strk commented Dec 16, 2016

@allandegnan this pr is about OpenID rather than OpenID Conenct. The difference is that while OpenID allows people to sign in using arbitrary URLS, OpenID Connect requires consumers to know providers in advance, restricting accessibility. I think there's an OpenID Connect extension allowing for discovery of providers, to do something similar to what OpenID does, but I've never seen an implmentation of such a system. Your UKHomeOffice usecase seems to be no different, is that correct ?

We still want to support OpenID Connect but should probably not replace OpenID.
See also #27

@allandegnan
Copy link

@strk That makes sense. Thanks for clearing up the nuances between the two standards.

@strk
Copy link
Member Author

strk commented Dec 16, 2016

Issues to keep in mind while implementing OpenID: https://sites.google.com/site/openidreview/issues

@strk
Copy link
Member Author

strk commented Dec 16, 2016

I love the way OpenID is implemented in GNUSocial. See an example: https://loadaverage.org/main/openid

How it goes:

  • From the login page you have a try OpenID login link
  • The OpenID login page asks for OpenID url and remember me option
  • If users is found by OpenID url, she's logged in
  • If user is not found, a, OpenID Account Setup page is shown, from here user can:
    • connect her OpenID URL to an existing account, in which case the corresponding password is requested
    • create a new user, in which case user and email are requested (defaulting to those communicated by the OpenID provider, if any.

// ErrDelegatedAuth is not a real error but notifies code that
// authentication is delegated to a second request.
type ErrDelegatedAuth struct {
OP string // OpenID Provider
Copy link
Member

Choose a reason for hiding this comment

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

Should probably be Provider or OpenIDProvider (I'm partial to the former)

@strk
Copy link
Member Author

strk commented Dec 20, 2016 via email

@tboerger tboerger added the pr/wip This PR is not ready for review label Dec 20, 2016
Handle OpenID redirection
Implement the verfication routes

... still all to be architectured
@willemvd
Copy link
Contributor

willemvd commented Jan 2, 2017

I'm searching through the different issues and pr's , but can find a final decision if OpenId Connect will be support or not.
Can somebody tell me if it is?

Oh and btw, found this library for OpenId Connect, maybe that might help :)
https://github.com/coreos/go-oidc

@strk
Copy link
Member Author

strk commented Jan 2, 2017 via email

@willemvd
Copy link
Contributor

willemvd commented Jan 2, 2017

Guess the answer to my question is a "no" then

To answer your question... no, because I don't have any Go knowledge

@strk
Copy link
Member Author

strk commented Jan 2, 2017 via email

@strk strk closed this Jan 5, 2017
@strk strk deleted the openid-gitea branch January 5, 2017 08:39
@strk strk mentioned this pull request Jan 8, 2017
@tboerger tboerger added reviewed/invalid and removed type/feature Completely new functionality. Can only be merged if feature freeze is not active. type/proposal The new feature has not been accepted yet but needs to be discussed first. lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. pr/wip This PR is not ready for review labels Jan 16, 2017
@tboerger tboerger removed this from the 1.1.0 milestone Jan 16, 2017
@go-gitea go-gitea locked and limited conversation to collaborators Nov 23, 2020
earl-warren pushed a commit to earl-warren/gitea that referenced this pull request Jun 24, 2023
- Resolves go-gitea#271
- Ensure that the adminstrator password is at least `MIN_PASSWORD_LENGTH`.
hazycora pushed a commit to hazycora/gitea that referenced this pull request Sep 26, 2023
- Resolves go-gitea#271
- Ensure that the adminstrator password is at least `MIN_PASSWORD_LENGTH`.

(cherry picked from commit 28cb04c3f5040980e716ce66cd5906f324257e02)
(cherry picked from commit 95371ebd92cd005e2d50a4754e60525cf6135b86)
(cherry picked from commit a134288ab6b0291082d913c4e22456b31af58af9)
(cherry picked from commit 4202f052cb32aec71a61dd2afd814035a9d85eea)
(cherry picked from commit 510b7467d3ee0bf346ad1843775affe1df0675ae)
(cherry picked from commit f3a6e1f121e89aaf608fd9890eaf06ed939d1006)
(cherry picked from commit f340508819866f355feec6d01b349fa7df29ace9)
(cherry picked from commit b891bb176d48c3855cc5b6e4573e7a337af9d382)
(cherry picked from commit 1a1bfc38cc7863f5cb3022560cacb2006d08f113)
(cherry picked from commit 083d5aefed10e54814c4438eabcd01973d305502)
(cherry picked from commit 4586096be9b6214058245da3227541866ea4312f)
(cherry picked from commit 039fa20cc8a5b50d5cc37de4503e8a9a80042bcc)
@delvh delvh added issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea and removed reviewed/invalid labels Oct 7, 2023
6543 pushed a commit to 6543-forks/gitea that referenced this pull request Feb 26, 2024
- Resolves go-gitea#271
- Ensure that the adminstrator password is at least `MIN_PASSWORD_LENGTH`.

(cherry picked from commit 28cb04c3f5040980e716ce66cd5906f324257e02)
(cherry picked from commit 95371ebd92cd005e2d50a4754e60525cf6135b86)
(cherry picked from commit a134288ab6b0291082d913c4e22456b31af58af9)
(cherry picked from commit 4202f052cb32aec71a61dd2afd814035a9d85eea)
(cherry picked from commit 510b7467d3ee0bf346ad1843775affe1df0675ae)
(cherry picked from commit f3a6e1f121e89aaf608fd9890eaf06ed939d1006)
(cherry picked from commit f340508819866f355feec6d01b349fa7df29ace9)
(cherry picked from commit b891bb176d48c3855cc5b6e4573e7a337af9d382)
(cherry picked from commit 1a1bfc38cc7863f5cb3022560cacb2006d08f113)
(cherry picked from commit 083d5aefed10e54814c4438eabcd01973d305502)
(cherry picked from commit 4586096be9b6214058245da3227541866ea4312f)
(cherry picked from commit 039fa20cc8a5b50d5cc37de4503e8a9a80042bcc)
(cherry picked from commit 3ec9cb5)
(cherry picked from commit 00be0eee3727130966c34a3b95b10f2af06ea2ec)
(cherry picked from commit a1566030025df8cc83d20cbe2b6fb0f87304a1a5)
(cherry picked from commit 4d305e77742c181f68cd24724dfc685723a41b31)
(cherry picked from commit 51e8f21202ea766d69a4b3c26f44c6db07f47844)
(cherry picked from commit 58e354c98e6b361f6d651ffdca3d5cb459adbf2f)
(cherry picked from commit 20405564f56775ba0f29a54c9a6eca8136d8ac99)
(cherry picked from commit 1d7f49568319cfa49e9c8338f2375432f4917739)
(cherry picked from commit d457b9c9111c04ffcd26ff859e2ad804697c2621)
(cherry picked from commit 72b54bc4cce030540310e50acc41ea789a1e5221)
(cherry picked from commit d7ce723e350d21ef42eba7b7013543e2ba6e0e17)
(cherry picked from commit ce5f863d5d3eff77b9736db453f0f9a65241c9bb)
(cherry picked from commit 324b9318acbf5e12be922ee7f8fc0f0fece1743a)
(cherry picked from commit fff11fc)
(cherry picked from commit d3fa04aa699883df9b227382190f57726c591cb8)
(cherry picked from commit d3b24691f389d863be834ccc8b2c8910b1614f30)
(cherry picked from commit 736dfab3ae943fb1b87a5468248c5d80887a5e7c)
(cherry picked from commit 8be95ef7f41c9e1d343a89cbfe67bdccc01df1f8)
(cherry picked from commit 0ce04d93a858a61d322750906629ce7da0e22116)
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
issue/not-a-bug The reported issue is the intended behavior or the problem is not inside Gitea
Projects
None yet
Development

Successfully merging this pull request may close these issues.

10 participants