Add Gitea Oauth #2622

techknowlogick · 2019-03-08T17:24:34Z

With fallback to basic auth if Gitea server does not support oauth

CLAassistant · 2019-03-08T17:24:42Z

All committers have signed the CLA.

bradrydzewski · 2019-03-08T17:54:58Z

awesome! one question: how does gitea handle the scenario where two separate go routines try to refresh the access token at the same time? this is something that caused issues in early gitlab oauth implementations, so just want to understand a bit more :)

techknowlogick · 2019-03-08T17:58:33Z

Tests fail because the above linked PR isn't merged

bradrydzewski · 2019-03-08T17:59:01Z

I just merged :)

techknowlogick · 2019-03-08T17:59:36Z

Thanks @bradrydzewski 😄

techknowlogick · 2019-03-08T18:04:42Z

@jonasfranz could you answer @bradrydzewski's question above?

techknowlogick · 2019-03-08T19:41:00Z

@bradrydzewski ~~looking through Gitea OAuth code, refreshing token isn't implemented in Gitea (yet).~~ I was able to compile drone and login with this PR and it pulled all the repos. I've also been able to submit an updated PR to go-login with being able to use RedirectURL.

cmd/drone-server/config/config.go

cmd/drone-server/inject_login.go

appleboy · 2019-03-09T02:56:25Z

also update the provideClient func:

    case config.Github.ClientID != "":
        return provideGithubClient(config)
-    case config.Gitea.Server != "":
+    case config.Gitea.ClientID != "":
        return provideGiteaClient(config)
    case config.GitLab.ClientID != "":
        return provideGitlabClient(config)

techknowlogick · 2019-03-09T02:57:41Z

@appleboy I kept config.Gitea.ClientID != "" codition so this doesn't break for Gitea users < 1.8 so they can still use username/password

appleboy · 2019-03-09T03:01:25Z

@techknowlogick Good catch. Thanks for the PR. LGTM.

jonasfranz · 2019-03-09T08:07:38Z

Refresh tokens are implemented. You can only use an access token or refresh token once to redeem a new token.

bradrydzewski · 2019-03-09T08:23:00Z

For context, GitLab had a similar restriction a few years ago that they since removed. Here are some issues that we ran into as a result #855

There are some other documented issues described in these threads:
doorkeeper-gem/doorkeeper#815
doorkeeper-gem/doorkeeper#449

It is very exciting to see oauth support land in Gitea (congrats!) and I look forward to taking advantage of this in Drone. However, we need to solve for these issues before we can merge this pull request, otherwise the current refresh token restrictions will break Drone.

cmd/drone-server/config/config.go

tboerger · 2019-03-09T15:31:50Z

Great to see that this gets finally integrated

bradrydzewski · 2019-03-13T20:43:11Z

cmd/drone-server/config/config.go

- Server string `envconfig:"DRONE_GITEA_SERVER"`
- SkipVerify bool `envconfig:"DRONE_GITEA_SKIP_VERIFY"`
- Debug bool `envconfig:"DRONE_GITEA_DEBUG"`
+ Server string `envconfig:"DRONE_GITEA_SERVER" default:"https://try.gitea.io"`


this default needs to be removed. An zero value server string is the only way we know if gitea is configured or not.

Good catch. Thanks :)

I've updated.

appleboy · 2019-03-14T01:53:24Z

@techknowlogick conflicts.

bradrydzewski

to merge this pull request, we need to address refresh token issues raised in my earlier comment #2622 (comment)

jonasfranz · 2019-04-11T05:05:43Z

@bradrydzewski do you propose to remove refresh token invalidation after a new token is signed?

bradrydzewski · 2019-04-11T05:13:30Z

@jonasfranz yes, I think invalidating existing refresh tokens after a new refresh token is generated causes more problems than it solves (see the doorkeeper issues I referenced above). Google and Bitbucket, for example, only invalidate refresh tokens if the user revokes access to the application. Or you could make refresh tokens optional (neither GitLab or GitHub use refresh tokens, although there are security benefits that need to be weighed).

lunny · 2019-04-12T07:57:57Z

@techknowlogick go.mod conflicted

jonasfranz · 2019-04-12T08:25:53Z

@bradrydzewski With my latest PR, Gitea will not invalidate refresh tokens by default. But there is an option to enable token invalidation.

bradrydzewski · 2019-04-12T08:28:00Z

@jonasfranz awesome, can you link to the pull request here. Once it is merged, we can merge this pull request. Also I will need to document the minimum version of Gitea that will be required -- can you let me know which version this will be? Thanks!!

jonasfranz · 2019-04-12T08:30:38Z

@bradrydzewski The PR is already merged: go-gitea/gitea#6584

It will be in version 1.8.0 (or 1.8.0-rc3 if we have a rc3).

techknowlogick · 2019-04-12T14:15:55Z

@lunny conflict resolved.

appleboy · 2019-04-13T00:50:26Z

@bradrydzewski Already merged in Gitea.

bradrydzewski · 2019-04-13T09:12:21Z

excellent. one last request -- can we get an entry for this in the changelog? thanks!

lunny · 2019-04-13T13:13:55Z

@bradrydzewski I think yes. This could be in release notes of v1.8.

bradrydzewski · 2019-04-13T15:10:40Z

what I mean is that we need an entry for this change in Drone's changelog file (in the unreleased section) https://github.com/drone/drone/blob/master/CHANGELOG.md

techknowlogick · 2019-04-13T15:37:52Z

@bradrydzewski added

tboerger · 2019-04-21T17:52:16Z

This works with gitea master? Or also with some stable release?

techknowlogick · 2019-04-21T17:58:03Z

@tboerger it works with 1.8.0 which was just released yesterday, but Drone can still work with older versions of gitea using basic auth.

oliver006 · 2019-04-21T22:38:23Z

I've been using it successfully with docker images gitea:v1.8.0-rc3 (and 1.8.0 now that it's released) and drone/drone@sha256:7acfbba60ac6c35537b099ce612aac0af3cdbfa2e48d0ee96730cd4c904b89fa (the sha of the image right after this PR got merged).

But there seems to be a little problem with refreshing the gitea token. After a while, I'm no longer to e.g. enable a gitea repo for drone. I'm still logged in and can modify settings within drone. Once I log out and back in, everything is fine.

Should I open a separate issue to track the problem?

techknowlogick · 2019-04-21T23:37:02Z

@oliver006 between rc3 and final gitea released a patch that should fix the issue you described above. Please feel free to open an issue in gitea tracker if this comes up again.

oliver006 · 2019-04-22T01:34:29Z

Thanks for the quick response @techknowlogick . I tried with Gitea 1.8.0 and the drone/drone image I mentioned above but it's still not working. I get:

I checked in the DB, the user_oauth_expiry value is unchanged (and 2h in the past).

I'll open an issue in the Gitea project.

[edit]
Opened: go-gitea/gitea#6703

bradrydzewski · 2019-04-22T18:17:28Z

I think the issue is that if Gitea uses a refresh token, a refresher needs to be configured. See https://github.com/drone/drone/blob/master/cmd/drone-server/inject_login.go#L166

frostieDE · 2019-04-24T11:46:00Z

This features needs some documentation, or am I missing something on the docs? 😄

ukos-git · 2019-04-28T13:47:47Z

here are my quick notes for installing:

go to your gitea instance application settings page: https://gitea.io/user/settings/applications
add a oauth instance with arbitrary id and redirect to the login page: https://gitea.io/login

you will get ClientID and ClientSecret:

add the generated values to your drone environement:

DRONE_GITEA_CLIENT_ID=XXX
DRONE_GITEA_CLIENT_SECRET=XXX

be sure to use latest gitea/gitea and drone/drone images (gitea version is displayed in ui bottom left. Today is Gitea Version: 345bc06)

Probably not enough for a PR to docs but better than no documentation.

Add Gitea Oauth

665398f

With fallback to basic auth if Gitea server does not support oauth

techknowlogick changed the title ~~WIP: Add Gitea Oauth~~ Add Gitea Oauth Mar 8, 2019

techknowlogick added 2 commits March 8, 2019 13:38

Update inject_login.go

63ba489

update gomod

33044a1

techknowlogick changed the title ~~Add Gitea Oauth~~ WIP: Add Gitea Oauth Mar 8, 2019

pass redirect URL to Gitea

e4f5498

techknowlogick changed the title ~~WIP: Add Gitea Oauth~~ Add Gitea Oauth Mar 8, 2019

appleboy reviewed Mar 9, 2019

View reviewed changes

cmd/drone-server/config/config.go Outdated Show resolved Hide resolved

appleboy reviewed Mar 9, 2019

View reviewed changes

cmd/drone-server/inject_login.go Show resolved Hide resolved

update per @appleboy feedback

b0f3ee3

update per @appleboy feedback

911f7c1

jonasfranz reviewed Mar 9, 2019

View reviewed changes

cmd/drone-server/config/config.go Show resolved Hide resolved

bradrydzewski reviewed Mar 13, 2019

View reviewed changes

techknowlogick and others added 2 commits March 13, 2019 18:27

Update config.go

39df2ff

update gomod

c797df0

Merge branch 'master' into gitea-oauth

dd0795b

techknowlogick added 2 commits March 14, 2019 18:14

Merge branch 'master' into gitea-oauth

bb8433e

Merge branch 'master' into gitea-oauth

8f351ea

bradrydzewski requested changes Apr 8, 2019

View reviewed changes

jonasfranz mentioned this pull request Apr 11, 2019

Add option to disable refresh token invalidation go-gitea/gitea#6584

Merged

Merge branch 'master' into gitea-oauth

236286e

Update CHANGELOG.md

1e44ae4

bradrydzewski merged commit 1a01a70 into harness:master Apr 13, 2019

techknowlogick deleted the gitea-oauth branch April 13, 2019 18:22

oliver006 mentioned this pull request Apr 22, 2019

OAuth Provider - getting "unauthorized" when trying to enable repo in drone go-gitea/gitea#6703

Closed

1 task

6543 mentioned this pull request Mar 19, 2021

OAuth support for Gitea woodpecker-ci/woodpecker#136

Closed

Add Gitea Oauth #2622

Add Gitea Oauth #2622

Conversation

techknowlogick commented Mar 8, 2019 • edited Loading

CLAassistant commented Mar 8, 2019 • edited Loading

bradrydzewski commented Mar 8, 2019

techknowlogick commented Mar 8, 2019

bradrydzewski commented Mar 8, 2019

techknowlogick commented Mar 8, 2019

techknowlogick commented Mar 8, 2019

techknowlogick commented Mar 8, 2019 • edited Loading

appleboy commented Mar 9, 2019

techknowlogick commented Mar 9, 2019

appleboy commented Mar 9, 2019

jonasfranz commented Mar 9, 2019 • edited Loading

bradrydzewski commented Mar 9, 2019 • edited Loading

tboerger commented Mar 9, 2019

bradrydzewski Mar 13, 2019 • edited Loading

Choose a reason for hiding this comment

techknowlogick Mar 13, 2019

Choose a reason for hiding this comment

appleboy commented Mar 14, 2019

bradrydzewski left a comment

Choose a reason for hiding this comment

jonasfranz commented Apr 11, 2019

bradrydzewski commented Apr 11, 2019 • edited Loading

lunny commented Apr 12, 2019

jonasfranz commented Apr 12, 2019

bradrydzewski commented Apr 12, 2019

jonasfranz commented Apr 12, 2019

techknowlogick commented Apr 12, 2019

appleboy commented Apr 13, 2019

bradrydzewski commented Apr 13, 2019

lunny commented Apr 13, 2019

bradrydzewski commented Apr 13, 2019 • edited Loading

techknowlogick commented Apr 13, 2019

tboerger commented Apr 21, 2019

techknowlogick commented Apr 21, 2019

oliver006 commented Apr 21, 2019 • edited Loading

techknowlogick commented Apr 21, 2019

oliver006 commented Apr 22, 2019 • edited Loading

bradrydzewski commented Apr 22, 2019

frostieDE commented Apr 24, 2019

ukos-git commented Apr 28, 2019

techknowlogick commented Mar 8, 2019 •

edited

Loading

CLAassistant commented Mar 8, 2019 •

edited

Loading

techknowlogick commented Mar 8, 2019 •

edited

Loading

jonasfranz commented Mar 9, 2019 •

edited

Loading

bradrydzewski commented Mar 9, 2019 •

edited

Loading

bradrydzewski Mar 13, 2019 •

edited

Loading

bradrydzewski commented Apr 11, 2019 •

edited

Loading

bradrydzewski commented Apr 13, 2019 •

edited

Loading

oliver006 commented Apr 21, 2019 •

edited

Loading

oliver006 commented Apr 22, 2019 •

edited

Loading