Login via OpenID-2.0 #618

strk · 2017-01-08T22:41:47Z

This branch superceeds the one in #271,
fixes #185 (once ready)

This version does not make "OpenID" a "Login Source"
but rather a first-class login method which is alternative
to the "username"/"password" pair.

The idea is to allow users to add their OpenID in the settings
and to allow registering via OpenID. Code is not yet complete.

strk · 2017-01-08T22:51:30Z

@stevenroose if you want to take a look at the new approach that'll help.

lunny · 2017-01-09T01:21:33Z

models/user.go

@@ -86,6 +86,7 @@ type User struct {
 	Orgs             []*User       `xorm:"-"`
 	Repos            []*Repository `xorm:"-"`
 	Location         string
+	OpenID           string `xorm:"UNIQUE"`


Then OpenID should be null default.

lunny · 2017-01-09T01:22:33Z

modules/setting/setting.go

@@ -689,6 +690,7 @@ please consider changing to GITEA_CUSTOM`)
 	CookieRememberName = sec.Key("COOKIE_REMEMBER_NAME").MustString("gitea_incredible")
 	ReverseProxyAuthUser = sec.Key("REVERSE_PROXY_AUTHENTICATION_USER").MustString("X-WEBAUTH-USER")
 	MinPasswordLength = sec.Key("MIN_PASSWORD_LENGTH").MustInt(6)
+	EnableOpenIDLogin = sec.Key("ENABLE_OPENID_LOGIN").MustBool(true)


For compatible consideration, default should be false.

lunny · 2017-01-09T01:24:58Z

Why not implemented as a login type like SMTP or PAM?

bkcsoft · 2017-01-09T01:31:06Z

I'm also not very happy about introducing new "first-class login-methods", should IMO be implemented like PAM/SMTP/LDAP. Unless you have a really good reason not to 😉

strk · 2017-01-09T07:55:18Z

On Sun, Jan 08, 2017 at 05:21:33PM -0800, Lunny Xiao wrote: > + OpenID string `xorm:"UNIQUE"` Then `OpenID` should be `null` default.

How do I specify that I want a `null` default ? That's what I want.

strk · 2017-01-09T07:56:07Z

On Sun, Jan 08, 2017 at 05:22:34PM -0800, Lunny Xiao wrote: + EnableOpenIDLogin = sec.Key("ENABLE_OPENID_LOGIN").MustBool(true) For compatible consideration, default should be false.

Enabling OpenID Login doesn't break any compatibility. Anyway it's still too early to deal with this, for now it helps developers/testers.

strk · 2017-01-09T07:59:24Z

On Sun, Jan 08, 2017 at 05:24:58PM -0800, Lunny Xiao wrote: Why not implemented as a login type like SMTP or PAM?

That's what the previous attempt tried to do, but the code to do that ended up being very convoluted: the code did not expect multi-step login process so I had to use an "ErrorDelegatedAuth" object as an hack to bend it in; it made no sense to have multiple "OpenID" login options (like it would to have multiple SMTP authentications); there was no easy way to handle the "failed login, try next thing" as OpenID does not have a username/password pair to pass on ...

…

--strk;

strk · 2017-01-09T08:00:59Z

On Sun, Jan 08, 2017 at 05:31:07PM -0800, bkcsoft wrote: I'm also not very happy about introducing new "first-class login-methods", should IMO be implemented like PAM/SMTP/LDAP. Unless you have a _really_ good reason not to 😉

As noted in the other answer, all of PAM/SMTP/LDAP rely on a username/password pair, and are all checked synchronously. OpenID behaves differently so abstracting it to look like PAM/SMTP/LDAP makes the code very convoluted with no obvious advantage. For example you still need a different form.

strk · 2017-01-09T08:02:14Z

Another thing: when you successfully authenticate via OpenID but no local user with such OpenID is known, the OpenID Login should prompt you to register as a new user OR "connect" your OpenID to an existing account, providing username/password to confirm it belongs to you. Once again this mechanism is very far from the PAM/SMTP/LDAP auth methods.

bkcsoft · 2017-01-09T08:26:58Z

How do I specify that I want a null default ? That's what I want.

xorm:"UNIQUE default null"

strk · 2017-01-09T08:32:30Z

On Mon, Jan 09, 2017 at 12:26:59AM -0800, bkcsoft wrote: > How do I specify that I want a `null` default ? That's what I want. `xorm:"UNIQUE default null"`

I've just checked and at least on PostgreSQL those field already have a default of null. Is that not the case for other backends ?

bkcsoft · 2017-01-09T08:38:49Z

I've just checked and at least on PostgreSQL those field already have a default of null. Is that not the case for other backends ?

Better to be specific than to depend on current behaviour :)

strk · 2017-01-09T09:37:13Z

TODO:

Check for OpenID validity before setting it in user settings
Add "Remember me" support upon login
Allow registering or connecting existing user signing in with valid but unknown OpenID
Do not leak nonce and discovery caches
Let admin specify whitelist/blacklist patterns for accepted/denied OpenID uris
Add support for OpenID 1 (wordpress.com - wordpress.com OpenID provider not supported yohcop/openid-go#43)

strk · 2017-01-10T20:13:57Z

For the record: openid.stackexchange.com works as a provider with this implementation

strk · 2017-01-13T17:44:32Z

For the record I've just tested this implementation against GNUSocial OpenID provider and it does work great :)

couling · 2017-01-15T14:11:16Z

Another thing: when you successfully authenticate via OpenID but no local user with such OpenID is known, the OpenID Login should prompt you to register as a new user OR "connect" your OpenID to an existing account, providing username/password to confirm it belongs to you. Once again this mechanism is very far from the PAM/SMTP/LDAP auth methods.

I'm less familiar with OpenID's internals. Does it provide an authenticated email address?

One thing I've seen when authenticating with my google account is that it acts as an alternative auth mechanism... IE once or twice I forgot that I'd originally signed up on a website with a username & password and later just clicked the "login with google" button. The result was that it logged me in without question (or asking my password).

Likewise many sites provide a streamlined account creation (skipping the email verification step) as google verifies the email.

strk · 2017-01-16T22:53:13Z

OpenID only provides proof of someone being in control of the content in an URI. Optionally provides some information about the URI "owner", like `nickname` and `email`, but if you do want a proof of the user really owning an email you'll have to verify that yourself (or trust the URI publisher to be in charge of those kind of checks). So: the registration process still needs to ask you for a username and an email (for which proposals may be provided via OpenID), signin would only need to ask you for an URI. If you have a stackexchange account you can already try this out, checkout this branch and enter "openid.stackexchange.com" as your OpenID login...

Likewise many sites provide a streamlined account creation (skipping the email verification step).

Again, this relies on trusting the Identity Provider in their checking user information, which is something you cannot really expect to be able to do when willing to let anyone run their own identity provider. NOTE in the current state of this branch there's no email checking (I hadn't looked if Gitea has support for those kind of verifications either).

lunny · 2017-03-17T09:28:41Z

Is the OpenID login could be disabled?

pgaskin · 2017-03-17T11:51:02Z

Yes. It is disabled by default and has an entry in the configuration file. On Mar 17, 2017 5:28 AM, Lunny Xiao <[email protected]> wrote: Is the OpenID login could be disabled? — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub<#618 (comment)>, or mute the thread<https://github.com/notifications/unsubscribe-auth/ANQ-1QdE0d4ckarvhzn8cIAhBIIdrCTCks5rmlJLgaJpZM4Ld07o>.

bkcsoft · 2017-03-17T14:13:28Z

Yeah it's good enought for now. We could iterate on it later if it becomes a pain 🙂

pgaskin · 2017-03-17T14:15:00Z

I am not sure if it is this PR or not, but I think this PR may be causing issues with GitHub OAuth2.

strk · 2017-03-17T15:28:00Z

@geek1011 I cannot reproduce issues with OAuth2 on my local system, please file a new ticket with more information about your setup

Quix0r · 2017-03-21T09:33:58Z

Just a side question: If I rebase my work, the PR relinks all commits and the code-reviewers have to work all over again. How can I prevent this from happening? Tons of merges (as I have to keep it in-sync with development branch) is surely working but not always wanted as it causes a complex GIT history.

And thank you @strk for your work. 👍

Quix0r · 2017-03-21T09:42:32Z

Is this installed at https://staging.geek1011.net/ ? Because when I try to login with my OpenID at https://social.mxchange.org/roland, I noticed two things:

redirection happens over http:// and not entirely https://
login is not given (not signup tried before)

cookies and JavaScript are allowed.

strk · 2017-03-22T09:49:00Z

This branch is actually merged to master now, and functionality is exposed on try.gitea.io

strk · 2017-03-22T09:50:18Z

@Quix0r please file a ticket if things don't work for you.
NOTE: I tried your OpenID url and was redirected for login to https://social.mxchange.org/main/login (so https)

strk · 2017-03-22T09:54:42Z

BTW: you might be interested in some OpenID merge requests I filed for GNUSocial, as I see your instance is also affected. Take a look at these:
https://git.gnu.io/gnu/gnu-social/merge_requests/139
https://git.gnu.io/gnu/gnu-social/merge_requests/138
https://git.gnu.io/gnu/gnu-social/issues/60 (fixed by merging openid/php-openid#134)

Quix0r · 2017-03-23T22:31:52Z

Okay, seems to work here: https://try.gitea.io/Quix0r So I guess it is not my instance (it is now pure GS code, without my changes).

danielfbm · 2017-12-26T03:34:49Z

Hi, just to raise a question in a related issue before creating a new one. Are there any plans to support OpenID Connect as well, or are we staying with OpenID 2.0?
Thanks in advance

Quix0r · 2017-12-29T22:00:18Z

Not working anymore: Application Version: 08cf7d9 will give a 500 error with enabled cookies and local storage (maybe not needed?)

strk · 2017-12-30T10:58:12Z

OpenID-Connect could be added as one of the flows supported by the OAuth2 provider. I don't think anyone is working on it but that's likely the plan, want to help ?

danielfbm · 2017-12-31T03:36:43Z

@strk I found it in Admin Console > Authentication > oAuth2 > OpenID Connect.

I tested with dex and it is working. Thanks

strk · 2017-12-31T08:15:16Z

ah, great, thanks for testing!

strk · 2017-12-31T08:16:20Z

Correct dex link, for the record: https://github.com/coreos/dex

sbrl · 2017-12-31T19:59:14Z

Umm that link to dex takes me to a gitea comparison, @danielfbm

danielfbm · 2018-01-01T03:12:11Z

@sbrl sorry, already updated the link

micheelengronne · 2020-04-08T10:59:21Z

Is it possible to force OpenID auth and to a certain provider ?

My usecase is as follow:

I would like to integrate Gitea in a more global solution containing other tools. I would like users to be able to connect through the OIDC server of this solution and going to gitea seamlessly.

So, I would like a way to redirect the Gitea login page to this specific OIDC server only in order to connect and use the complete sets of OIDC RFCs to manage sessions, user logout (frontend and backend), user lifecycle (insert, update, delete, frontend and backend).

Thanks.

lafriks · 2020-04-08T11:23:34Z

Please do not comment on old merged PR. For feature requests open new issues for questions please use our discord channel or gitea forum https://discourse.gitea.io/

strk force-pushed the openid branch from 2788b2d to 3bdd1d1 Compare January 8, 2017 22:43

tboerger added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Jan 8, 2017

lunny reviewed Jan 9, 2017

View reviewed changes

lunny added this to the 1.1.0 milestone Jan 9, 2017

lunny added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Jan 9, 2017

strk force-pushed the openid branch from 3bdd1d1 to 0d5bb15 Compare January 9, 2017 08:14

strk force-pushed the openid branch from ccdcb50 to 9f9dc2a Compare January 12, 2017 09:02

lunny modified the milestones: 1.2.0, 1.1.0 Jan 15, 2017

Add some info about OpenID

8c2363d

bkcsoft approved these changes Mar 17, 2017

View reviewed changes

bkcsoft merged commit 71d16f6 into go-gitea:master Mar 17, 2017

pgaskin mentioned this pull request Mar 17, 2017

Fix inconsistency in layout #1316

Merged

strk deleted the openid branch March 18, 2017 13:12

strk mentioned this pull request Mar 20, 2017

Support federated login for guest users (OpenID) #185

Closed

strk mentioned this pull request Sep 26, 2017

[CHANGELOG] Drop redundant OpenID items #2608

Closed

Lichtjaeger mentioned this pull request Oct 16, 2017

Wrong method to fetch token for openID connect #2421

Closed

7 tasks

go-gitea locked as resolved and limited conversation to collaborators Apr 8, 2020

Login via OpenID-2.0 #618

Login via OpenID-2.0 #618

Conversation

strk commented Jan 8, 2017

strk commented Jan 8, 2017

lunny Jan 9, 2017

Choose a reason for hiding this comment

lunny Jan 9, 2017

Choose a reason for hiding this comment

lunny commented Jan 9, 2017

bkcsoft commented Jan 9, 2017

strk commented Jan 9, 2017 via email

strk commented Jan 9, 2017 via email

strk commented Jan 9, 2017 via email

strk commented Jan 9, 2017 via email

strk commented Jan 9, 2017 via email

bkcsoft commented Jan 9, 2017

strk commented Jan 9, 2017 via email

bkcsoft commented Jan 9, 2017

strk commented Jan 9, 2017 • edited Loading

strk commented Jan 10, 2017

strk commented Jan 13, 2017

couling commented Jan 15, 2017 • edited Loading

strk commented Jan 16, 2017 via email

lunny commented Mar 17, 2017

pgaskin commented Mar 17, 2017 via email

bkcsoft commented Mar 17, 2017

pgaskin commented Mar 17, 2017

strk commented Mar 17, 2017 via email

Quix0r commented Mar 21, 2017 • edited Loading

Quix0r commented Mar 21, 2017

strk commented Mar 22, 2017

strk commented Mar 22, 2017

strk commented Mar 22, 2017

Quix0r commented Mar 23, 2017

danielfbm commented Dec 26, 2017

Quix0r commented Dec 29, 2017

strk commented Dec 30, 2017 via email

danielfbm commented Dec 31, 2017 • edited Loading

strk commented Dec 31, 2017

strk commented Dec 31, 2017

sbrl commented Dec 31, 2017

danielfbm commented Jan 1, 2018

micheelengronne commented Apr 8, 2020

lafriks commented Apr 8, 2020

strk commented Jan 9, 2017 •

edited

Loading

couling commented Jan 15, 2017 •

edited

Loading

Quix0r commented Mar 21, 2017 •

edited

Loading

danielfbm commented Dec 31, 2017 •

edited

Loading