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

Add API endpoints to manage OAuth2 Application (list/create/delete) #10437

Merged
merged 15 commits into from
Feb 29, 2020

Conversation

ghost
Copy link

@ghost ghost commented Feb 24, 2020

This PR adds support to create OAuth2 Applications through the API. The intention is to be able to automate the creation of application programmatically.

Maybe it is related to the issue #8764 and #9210

Tasks:

  • Create OAuth2 application
  • List existing OAuth2 applications (without secrets)
  • Delete an OAuth2 application by its ID
  • Add tests for create/list/delete

routers/api/v1/user/app.go Outdated Show resolved Hide resolved
@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Feb 24, 2020
@ghost
Copy link
Author

ghost commented Feb 24, 2020

Added List and Delete methods as requested.

@ghost ghost force-pushed the feat/api-oauth2-applications branch 2 times, most recently from 6e4d549 to b179bfa Compare February 24, 2020 15:37
@ghost ghost changed the title add API endpoint to create OAuth2 Application. add API endpoint to manage OAuth2 Application (list/create/delete) Feb 24, 2020
@ghost ghost changed the title add API endpoint to manage OAuth2 Application (list/create/delete) adds API endpoints to manage OAuth2 Application (list/create/delete) Feb 24, 2020
@ghost ghost requested review from lunny and adelowo February 24, 2020 15:44
@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 Feb 24, 2020
@techknowlogick techknowlogick added the type/feature Completely new functionality. Can only be merged if feature freeze is not active. label Feb 24, 2020
@techknowlogick techknowlogick added this to the 1.12.0 milestone Feb 24, 2020
models/oauth2_application.go Outdated Show resolved Hide resolved
Copy link
Member

@6543 6543 left a comment

Choose a reason for hiding this comment

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

great work @IGZgustavomarin only smal nits left and we need some tests

routers/api/v1/user/app.go Outdated Show resolved Hide resolved
@ghost ghost force-pushed the feat/api-oauth2-applications branch from 9521b76 to 7d2b16e Compare February 24, 2020 19:00
@ghost
Copy link
Author

ghost commented Feb 24, 2020

great work @IGZgustavomarin only smal nits left and we need some tests

this is a big project, I've tried to setup my environment for testing but couldn't do it. I'll try again. But any help is welcome.

Also not sure which criteria to follow add tests. Which methods of this PR should be covered?

NOTE: Although, I did manual tests by building/running as a local docker image and sending requests with Postman.

@ghost ghost force-pushed the feat/api-oauth2-applications branch from 7d2b16e to 2f7d6c8 Compare February 24, 2020 19:10
@techknowlogick
Copy link
Member

CI Fail with:

modules/convert/convert.go:392:1: exported function ToOAuth2Application should have comment or be unexported

@ghost
Copy link
Author

ghost commented Feb 24, 2020

CI Fail with:

modules/convert/convert.go:392:1: exported function ToOAuth2Application should have comment or be unexported

fixed

routers/api/v1/swagger/app.go Outdated Show resolved Hide resolved
routers/api/v1/swagger/app.go Outdated Show resolved Hide resolved
@ghost ghost force-pushed the feat/api-oauth2-applications branch 2 times, most recently from aa6c31c to f4120f4 Compare February 24, 2020 19:45
@codecov-io
Copy link

codecov-io commented Feb 24, 2020

Codecov Report

Merging #10437 into master will decrease coverage by <.01%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #10437      +/-   ##
==========================================
- Coverage   43.75%   43.75%   -0.01%     
==========================================
  Files         586      586              
  Lines       81633    81633              
==========================================
- Hits        35717    35716       -1     
  Misses      41492    41492              
- Partials     4424     4425       +1
Impacted Files Coverage Δ
modules/process/manager.go 74.69% <0%> (-3.62%) ⬇️
modules/git/command.go 86.95% <0%> (-2.61%) ⬇️
modules/log/file.go 75.52% <0%> (-2.1%) ⬇️
modules/queue/unique_queue_disk_channel.go 53.84% <0%> (-1.93%) ⬇️
services/pull/check.go 50% <0%> (-1.22%) ⬇️
services/pull/pull.go 34.89% <0%> (-0.3%) ⬇️
models/error.go 29.23% <0%> (+0.51%) ⬆️
modules/git/repo.go 47.7% <0%> (+0.91%) ⬆️
modules/git/utils.go 70.14% <0%> (+4.47%) ⬆️
modules/indexer/stats/db.go 50% <0%> (+9.37%) ⬆️
... and 1 more

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 e979cb6...3d0c072. Read the comment docs.

@ghost
Copy link
Author

ghost commented Feb 25, 2020

Kudos to @6543 for all the time you spent helping me to implement a better features and the tests.

@lunny
Copy link
Member

lunny commented Feb 25, 2020

@IGZgustavomarin CI failed.

@ghost
Copy link
Author

ghost commented Feb 25, 2020

@IGZgustavomarin CI failed.

Yes, but it fails on different places on each pipeline, and none on TestOAuth2Application features that this PR contains. This are the error messages:

First on testing-amd64 > test-mysql >:

RepoAssignment Invalid repo /go/src/code.gitea.io/gitea/integrations/gitea-integration-mysql/gitea-repositories/user2/repo1.git: no such file or directory
--
314 | release_test.go:65:
315 | Error Trace:	integration_test.go:400
316 | integration_test.go:258
317 | release_test.go:65
318 | Error:      	Not equal:
319 | expected: 200
320 | actual  : 500
321 | Test:       	TestViewReleases
322 | Messages:   	Request: GET /user2/repo1/releases
323 | testlogger.go:73: 2020/02/25 13:02:39 ...ers/routes/routes.go:105:func1() [I] Completed GET /user2/repo1/releases 500 Internal Server Error in 6.060573ms

And then on testing-arm64 > test-sqlite >:

[E] DeleteRepository: database is deadlocked
--
287 | testlogger.go:73: 2020/02/25 12:56:04 ...ers/routes/routes.go:105:func1() [I] Completed DELETE /api/v1/repos/user2/repo-tmp-17?token=fc86029a7982c983a0505fa36c003dce11bcfad8 500 Internal Server Error in 94.444323ms

It doesn't seem related to this feature. Can the tests be run again? Or, in caseyou think this is related with this code, how to start debugging it.

Thanks!

@ghost ghost force-pushed the feat/api-oauth2-applications branch from 07d51c3 to c296222 Compare February 25, 2020 16:16
@6543
Copy link
Member

6543 commented Feb 25, 2020

@IGZgustavomarin if it CI fail unrelated the easyest way to restart is to push an empty commit: git commit -m 'CI restart' --allow-empty & git push ...

@ghost
Copy link
Author

ghost commented Feb 25, 2020

@IGZgustavomarin if it CI fail unrelated the easyest way to restart is to push an empty commit: git commit -m 'CI restart' --allow-empty & git push ...

There were some changes on master branch so I just rebased and pushed again. It seems all green now.

@6543
Copy link
Member

6543 commented Feb 25, 2020

the most convenient methot to keep your pull up to date is the "update branch" botton of github
and since a pull is squash-merged it doesnt mather in git history anyway

@6543
Copy link
Member

6543 commented Feb 25, 2020

ping lgtm

@6543
Copy link
Member

6543 commented Feb 27, 2020

@lunny @adelowo this pull need your review or review-dismiss

@guillep2k
Copy link
Member

Ping LG-TM

@guillep2k guillep2k merged commit af61b22 into go-gitea:master Feb 29, 2020
@zeripath zeripath changed the title adds API endpoints to manage OAuth2 Application (list/create/delete) Add API endpoints to manage OAuth2 Application (list/create/delete) May 17, 2020
@go-gitea go-gitea locked and limited conversation to collaborators Nov 24, 2020
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/feature Completely new functionality. Can only be merged if feature freeze is not active.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

7 participants