-
-
Notifications
You must be signed in to change notification settings - Fork 5.5k
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
Conversation
Fixes #185 |
1e51a25
to
8c4784d
Compare
7bbaea1
to
174a949
Compare
@@ -100,6 +100,19 @@ func SignIn(ctx *context.Context) { | |||
ctx.HTML(200, tplSignIn) | |||
} | |||
|
|||
// OpenidVerify handles response from OpenID provider | |||
func OpenidVerify(ctx *context.Context) { |
There was a problem hiding this comment.
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 |
There was a problem hiding this comment.
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?
Renamed, although consistency is not the immediate problem
with this work ...
I wonder if OpenID should just be optionally associated to users
who do also have another way to log in, as done with GNUSocial
and Friendica, for example. I'm not sure if auto-registration
makes sense with OpenID as you still need to provide a username
just OpenID would replace your password, basically.
|
@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). |
I think you'll still want to create a local user for "guests"
so to be able to grant them special access to some repos.
The OpenID auth method is just a facility for those guests so they
don't need to remember yet another password.
The "registration" phase is still needed because you want:
1) To assign a username (for referencing)
2) To store an email (for notification)
3) To remember the user preferences
I think what should happen in the "OpenID Login" page is that
if your OpenID url is valid but not yet registered you're are asked
to fill up the form (username/email/captcha?) and then you're in.
For an OpenID url to be valid I guess we'd also want it not to
be in a blacklist, or to be in a whitelist if given.
|
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. |
@allandegnan this pr is about We still want to support |
@strk That makes sense. Thanks for clearing up the nuances between the two standards. |
Issues to keep in mind while implementing OpenID: https://sites.google.com/site/openidreview/issues |
I love the way How it goes:
|
// ErrDelegatedAuth is not a real error but notifies code that | ||
// authentication is delegated to a second request. | ||
type ErrDelegatedAuth struct { | ||
OP string // OpenID Provider |
There was a problem hiding this comment.
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)
If we take the "dedicated OP login" approach this ErrDelegatedAuth
would not be needed. It was just an hack to force OpenID into the
chain of "login sources".
|
Handle OpenID redirection Implement the verfication routes ... still all to be architectured
I'm searching through the different issues and pr's , but can find a final decision if OpenId Connect will be support or not. Oh and btw, found this library for OpenId Connect, maybe that might help :) |
On Mon, Jan 02, 2017 at 03:21:53AM -0800, willemvd wrote:
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?
Free software is driven by do-ocracy.
OpenID Connect will be available if anyone will code it.
Are you up for it ?
|
Guess the answer to my question is a "no" then To answer your question... no, because I don't have any Go knowledge |
Your question was about decisions being taken, personally
I think decision have no weight until backed up by actual work.
To my knowledge there's no code yet for "OpenID Connect" support,
but I've heard someone mentioning they'd want to implement it.
You may file an issue suggesting it and see if anyone jumps in
to provide code.
|
- Resolves go-gitea#271 - Ensure that the adminstrator password is at least `MIN_PASSWORD_LENGTH`.
- 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)
- 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)
Far from ready, but open for contribution (mostly needs
UI and UX)