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

Multiple permissions fixes for projects #12547

Conversation

wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor

As communicated in discord/develop, here's this little PR.
I believe there is no reason to show the 'New Project board' button for users that are not signed in.
Before:
before
After:
after

@codecov-commenter
Copy link

codecov-commenter commented Aug 20, 2020

Codecov Report

Merging #12547 into master will decrease coverage by 0.01%.
The diff coverage is 66.66%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master   #12547      +/-   ##
==========================================
- Coverage   43.45%   43.44%   -0.02%     
==========================================
  Files         643      643              
  Lines       71144    71155      +11     
==========================================
- Hits        30919    30912       -7     
- Misses      35218    35233      +15     
- Partials     5007     5010       +3     
Impacted Files Coverage Δ
routers/repo/projects.go 13.19% <18.18%> (+0.21%) ⬆️
routers/routes/routes.go 88.92% <100.00%> (+0.02%) ⬆️
modules/indexer/stats/db.go 52.17% <0.00%> (-17.40%) ⬇️
modules/git/utils.go 73.77% <0.00%> (-3.28%) ⬇️
modules/log/event.go 57.54% <0.00%> (-1.89%) ⬇️
modules/git/repo.go 49.23% <0.00%> (-0.51%) ⬇️

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 a048489...465dadf. Read the comment docs.

@GiteaBot GiteaBot added the lgtm/need 2 This PR needs two approvals by maintainers to be considered for merging. label Aug 20, 2020
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf force-pushed the hide-for-logged-off-users_button-new-project-board branch 2 times, most recently from c845258 to 2d9d97f Compare August 20, 2020 19:43
@techknowlogick techknowlogick added this to the 1.13.0 milestone Aug 20, 2020
@techknowlogick techknowlogick added the topic/ui Change the appearance of the Gitea UI label Aug 20, 2020
templates/repo/projects/view.tmpl Outdated Show resolved Hide resolved
* there is no reason to show the button for users that are not signed in
@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 Aug 20, 2020
Copy link
Contributor

@zeripath zeripath left a comment

Choose a reason for hiding this comment

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

This is not the correct test - we should only show the button if the user can create project boards

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor Author

This is not the correct test - we should only show the button if the user can create project boards

This should now be addressed by the latest changes.
Button is not shown for unauthorized users nor if the repo is archived.

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor Author

projectboardmenu
Only show this dropdown menu to authorized users (same as the button).
as per f60df0d

* CanWriteIssues and CanWritePulls implies (and requires) signed in user
@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 Aug 21, 2020
@silverwind
Copy link
Member

This kind of screams for a CanCreateProject imho 😉

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor Author

This kind of screams for a CanCreateProject imho wink

Yeah, a similar thought also appeared in the discord. I am not sure how to do that, though. :(

@lafriks
Copy link
Member

lafriks commented Aug 21, 2020

adding new ctx variable is an option:

ctx.Data["PageIsProjects"] = true

@silverwind
Copy link
Member

silverwind commented Aug 21, 2020

Ideally it would be a utility function somewhere in the project-related files which is then called to set the template variable on pages that render that template. That way, it can be re-used everywhere.

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor Author

Ideally it would be a utility function somewhere in the project-related files which is then called to set the template variable on pages that render that template. That way, it can be re-used everywhere.

I may try to hack something together 🚀

@lunny
Copy link
Member

lunny commented Aug 21, 2020

I think we have a new unit named Project. Then those who have write permission of unit Project could edit and delete the board.

@zeripath
Copy link
Contributor

I've taken the liberty to do the fixes.

@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor Author

I've taken the liberty to do the fixes.

From what I can gather it looks great, thank you!

@techknowlogick techknowlogick merged commit d4e35b9 into go-gitea:master Aug 22, 2020
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf
Copy link
Contributor Author

Cheers folks! 🚀 🚀 🚀

@zeripath zeripath changed the title Hide 'New Project board' button for users that are not signed in Multiple permissions fixes for projects Aug 22, 2020
@zeripath zeripath added the topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! label Aug 22, 2020
@lafriks lafriks added the skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. label Aug 22, 2020
@wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf wULLSnpAXbWZGYDYyhWTKKspEQoaYxXyhoisqHf deleted the hide-for-logged-off-users_button-new-project-board branch August 22, 2020 13:29
@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. skip-changelog This PR is irrelevant for the (next) changelog, for example bug fixes for unreleased features. topic/security Something leaks user information or is otherwise vulnerable. Should be fixed! topic/ui Change the appearance of the Gitea UI
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

9 participants