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

Fix creating OAuth2 auth source from CLI #14116

Merged
merged 7 commits into from
Dec 24, 2020

Conversation

daniil-pankratov
Copy link
Contributor

This MR fixing creation OAuth2 auth source from CLI.
Problem is described here: #8356

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Dec 23, 2020
@codecov-io
Copy link

Codecov Report

Merging #14116 (0497101) into master (4d22e24) will increase coverage by 0.00%.
The diff coverage is 18.75%.

Impacted file tree graph

@@           Coverage Diff           @@
##           master   #14116   +/-   ##
=======================================
  Coverage   42.34%   42.34%           
=======================================
  Files         726      726           
  Lines       77839    77852   +13     
=======================================
+ Hits        32960    32967    +7     
- Misses      39462    39469    +7     
+ Partials     5417     5416    -1     
Impacted Files Coverage Δ
modules/auth/oauth2/oauth2.go 27.27% <0.00%> (-0.36%) ⬇️
routers/user/auth.go 11.97% <0.00%> (-0.08%) ⬇️
models/oauth2.go 34.14% <50.00%> (+0.81%) ⬆️
modules/git/tree_entry_nogogit.go 87.50% <0.00%> (-6.25%) ⬇️
modules/process/manager.go 72.50% <0.00%> (-2.50%) ⬇️
models/error.go 38.98% <0.00%> (+0.48%) ⬆️
services/pull/pull.go 42.85% <0.00%> (+0.50%) ⬆️
modules/indexer/stats/db.go 48.00% <0.00%> (+8.00%) ⬆️
modules/indexer/stats/queue.go 76.47% <0.00%> (+11.76%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 30edcd5...0497101. Read the comment docs.

models/oauth2.go Outdated Show resolved Hide resolved
Copy link
Member

@noerw noerw left a comment

Choose a reason for hiding this comment

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

Thanks for the PR! (fixes #8356 and #12938)

It would be nicer if goth providers were updated when adding a provider via CLI, instead of an error fallback during login.
The root of the issue is, that the CLI modifies the state of goth in the CLI process, not in the long-running gitea process.
Currently there is no RPC mechanism for this, we could add an API endpoint POST /api/v1/admin/auth-source for this.

As a first stopgap for the bug this still looks good to me.

@GiteaBot GiteaBot added lgtm/need 1 This PR needs approval from one additional maintainer to be merged. and removed lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. labels Dec 24, 2020
@GiteaBot GiteaBot added lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. and removed lgtm/need 1 This PR needs approval from one additional maintainer to be merged. labels Dec 24, 2020
@zeripath
Copy link
Contributor

make lg-tm work

@zeripath zeripath merged commit 5a94db3 into go-gitea:master Dec 24, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Feb 11, 2021
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
lgtm/done This PR has enough approvals to get merged. There are no important open reservations anymore. type/bug
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants