(auth): add oauth login #6

Merged
fnetX merged 7 commits from unkonstant/moderation:oauth into main 2 months ago

this adds an alternative way to login via an oauth app in gitea following https://docs.gitea.io/en-us/oauth2-provider/

this can be seen as a first implementation towards #5

currenlty only users which have the admin flag in gitea will be given the mod permission

this adds an alternative way to login via an oauth app in gitea following https://docs.gitea.io/en-us/oauth2-provider/ this can be seen as a first implementation towards https://codeberg.org/Codeberg/moderation/issues/5 currenlty only users which have the admin flag in gitea will be given the mod permission
unkonstant added 1 commit 4 months ago
5b275c3776 (auth): add oauth login
Collaborator

Oh, bad timing, I just pushed changes and didn't notice your PR. Will have a look at the conflicts in a second.

By the way, is there a way for local testing with this patch?

Oh, bad timing, I just pushed changes and didn't notice your PR. Will have a look at the conflicts in a second. By the way, is there a way for local testing with this patch?
Collaborator

Okay, thank you very much.

We are using files per feature, so for example the user.go is responsible mostly for interacting with users on the Gitea side.

I'd recommend moving the OAuth stuff to some oauth.go or auth.go file, or maybe into an own package if this is better.

Since I know what I just did, I think I'll try to open a PR to your branch to resolve the conflicts I just created, sorry again.

Okay, thank you very much. We are using files per feature, so for example the user.go is responsible mostly for interacting with users on the Gitea side. I'd recommend moving the OAuth stuff to some oauth.go or auth.go file, or maybe into an own package if this is better. Since I know what I just did, I think I'll try to open a PR to your branch to resolve the conflicts I just created, sorry again.
Collaborator

Thank you very much, I tested it and it seems to work great, even with localhost, so far.

See unkonstant/moderation#1

Thank you very much, I tested it and it seems to work great, even with localhost, so far. See https://codeberg.org/unkonstant/moderation/pulls/1
Collaborator

Huh, conflicts? I can't reproduce them locally. I suppose that Gitea didn't properly re-check upon the merge?

Huh, conflicts? I can't reproduce them locally. I suppose that Gitea didn't properly re-check upon the merge?
fnetX changed target branch from main to wip-show-delta 4 months ago
fnetX changed target branch from wip-show-delta to main 4 months ago
Collaborator

Huh, conflicts? I can't reproduce them locally. I suppose that Gitea didn't properly re-check upon the merge?

Push an empty commit to the PR, that will let the conflict checking work again.

> Huh, conflicts? I can't reproduce them locally. I suppose that Gitea didn't properly re-check upon the merge? > Push an empty commit to the PR, that will let the conflict checking work again.
Poster

I guess that could be related to https://github.com/go-gitea/gitea/issues/17204 (potential fix in https://github.com/go-gitea/gitea/pull/18658)?

Will do a push later.

I guess that could be related to https://github.com/go-gitea/gitea/issues/17204 (potential fix in https://github.com/go-gitea/gitea/pull/18658)? Will do a push later.
unkonstant added 1 commit 3 months ago
46c2800692 move oauth to own file
unkonstant added 1 commit 3 months ago
95ca242778 Merge branch 'main' into oauth
unkonstant added 1 commit 3 months ago
1150eac694 use logger for permission debug
unkonstant added 1 commit 3 months ago
4aec64a828 fix lint errors
unkonstant added 1 commit 3 months ago
dfc0c46c88 fix lint
Poster

@fnetX finally got this 🟢 , did not really have a chance to test again though after the merge. Please have a look.

@fnetX finally got this 🟢 , did not really have a chance to test again though after the merge. Please have a look.
Collaborator

@unkonstant Thank you. I suppose you won't find much time to improve this and keep up on potentially necessary changes to this code?

So I propose that we merge the code in as-is, and do further adjustments ourselves, unless you are interested in doing some further patches yourself :)

@unkonstant Thank you. I suppose you won't find much time to improve this and keep up on potentially necessary changes to this code? So I propose that we merge the code in as-is, and do further adjustments ourselves, unless you are interested in doing some further patches yourself :)
Poster

@fnetX 👍 with merging as is. I could do more on the feature itself in follow up PRs, the pace would be similar to this PR though. So if you are fine with moving slow I can still continue on this.

I would need a bit more input on how exactly this should move forward. The description in #5 is a bit vague on the mapping.

@fnetX 👍 with merging as is. I could do more on the feature itself in follow up PRs, the pace would be similar to this PR though. So if you are fine with moving slow I can still continue on this. I would need a bit more input on how exactly this should move forward. The description in #5 is a bit vague on the mapping.
Gusted reviewed 3 months ago
}
func randomString(n int) string {
letters := []rune("abcdefghijklmnopqrstuvwxyzABCDEFGHIJKLMNOPQRSTUVWXYZ0123456789")
Collaborator

Could we use a bit more secure version of this? E.g. fa5dec966c/modules/util/util.go (L168-L175) and then convert it via hex or base64 to a string?

Could we use a _bit more_ secure version of this? E.g. https://github.com/go-gitea/gitea/blob/fa5dec966c49207a2e1bc4c27feed69eaa69e44f/modules/util/util.go#L168-L175 and then convert it via hex or base64 to a string?
fnetX added the due date 2022-05-28 3 months ago
Collaborator

We'll merge this sooner or later, and eventually patch later.

@unkonstant I added some more thoughts in #5.

We'll merge this sooner or later, and eventually patch later. @unkonstant I added some more thoughts in #5.
fnetX merged commit f7a9773833 into main 2 months ago
fnetX referenced this issue from a commit 2 months ago
unkonstant deleted branch oauth 1 month ago
ci/woodpecker/pr/backend Pipeline was successful
The pull request has been merged as f7a9773833.
Sign in to join this conversation.
Loading…
There is no content yet.