Skip to content

Add regenerate secret feature for oauth2#6291

Merged
jonasfranz merged 2 commits into
go-gitea:masterfrom
jonasfranz:feature/regenerate-secret
Mar 9, 2019
Merged

Add regenerate secret feature for oauth2#6291
jonasfranz merged 2 commits into
go-gitea:masterfrom
jonasfranz:feature/regenerate-secret

Conversation

@jonasfranz

Copy link
Copy Markdown
Member

No description provided.

@jonasfranz jonasfranz added this to the 1.8.0 milestone Mar 9, 2019
@jonasfranz jonasfranz added type/enhancement An improvement of existing functionality type/feature Completely new functionality. Can only be merged if feature freeze is not active. and removed type/enhancement An improvement of existing functionality labels Mar 9, 2019
@GiteaBot GiteaBot added the lgtm/need 1 This PR needs approval from one additional maintainer to be merged. label Mar 9, 2019
Comment thread routers/user/setting/oauth2.go
return
}
ctx.Flash.Success(ctx.Tr("settings.update_oauth2_application_success"))
ctx.HTML(200, tplSettingsOAuthApplications)

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I would like we redirect to the oauth2 application edit page but not render it again so that it's safe to refresh the current page.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

This won't work since the secret is not saved and will only be shown once.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

Don't familiar with macaron and I don't know if it supports what I said. I will give this a LGTM and so that my opinion will not block this.

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

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

The only way would be via session but that would complicate the procedure a lot and it will be less secure since the token is saved in ram / session provider.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

There is temporary session variables that allow this work for redirects

@codecov-io

Copy link
Copy Markdown

Codecov Report

Merging #6291 into master will decrease coverage by 0.01%.
The diff coverage is 4%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #6291      +/-   ##
==========================================
- Coverage   38.89%   38.88%   -0.02%     
==========================================
  Files         359      359              
  Lines       50993    51018      +25     
==========================================
+ Hits        19833    19836       +3     
- Misses      28290    28312      +22     
  Partials     2870     2870
Impacted Files Coverage Δ
routers/user/setting/oauth2.go 0% <0%> (ø) ⬆️
routers/routes/routes.go 83.08% <100%> (+0.02%) ⬆️
models/repo_list.go 67.89% <0%> (+1.05%) ⬆️

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 8211e01...c388948. Read the comment docs.

@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 Mar 9, 2019
@jonasfranz jonasfranz merged commit 8fffb06 into go-gitea:master Mar 9, 2019
@jonasfranz jonasfranz deleted the feature/regenerate-secret branch March 9, 2019 16:30
@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.

6 participants