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

User with Staff role can approve contracts #3736

Merged
merged 1 commit into from
Jan 30, 2024

Conversation

frjo
Copy link
Contributor

@frjo frjo commented Jan 25, 2024

Fixes #3735

@frjo frjo changed the title Staff can approve contracts, no other. User with Staff role can approve contracts, no other. Jan 25, 2024
@frjo
Copy link
Contributor Author

frjo commented Jan 25, 2024

ARDC had an issue with this. A user with contracting + staff role was not allowed to approve contracts.

I think it should be ok to let any user with staff role approve contracts.

@@ -34,7 +34,7 @@ def can_approve_contract(user, project, **kwargs):
if not user.is_authenticated:
return False, "Login Required"

if user.is_apply_staff and not user.is_contracting and not user.is_applicant:
if user.is_apply_staff:
return True, "Only Staff can approve the contract"
Copy link
Member

Choose a reason for hiding this comment

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

If removing is_applicant then maybe ensure that the user approving is not applicant or partner on the project? I am assuming it's going be rare but just thinking around least privilege point of view.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Good point.

@frjo frjo force-pushed the fix/logic_in_can_approve_contract branch from 2090c62 to a1727e7 Compare January 25, 2024 20:05
Copy link
Member

@sandeepsajan0 sandeepsajan0 left a comment

Choose a reason for hiding this comment

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

Back then it was the requirement to hide it from Staff+contracting role because the user with Staff + contracting role can upload the contract and also can approve the contract but as of now, we have provided an option of uploading the contract to the staff as well so I think it is good to loosen up the approve permission as well.

@frjo frjo changed the title User with Staff role can approve contracts, no other. User with Staff role can approve contracts Jan 29, 2024
@frjo frjo added Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter labels Jan 29, 2024
@frjo frjo merged commit 6265420 into main Jan 30, 2024
10 checks passed
theskumar added a commit that referenced this pull request Jan 30, 2024
* main:
  User with Staff role can approve contracts (#3736)
  Add server side previews to the application workflow (#3725)
  Move cookieconsent settings to generic settings (#3722)
  Remove public "standard_pages" app (#3721)
  Remove public "funds" app (#3720)
  Update github actions. (#3740)
  Remove public "forms" pages app (#3719)
  Remove public "projects" app (#3718)
  Remove "people" app (#3739)
  Remove public "partners" app (#3716)
@frjo frjo deleted the fix/logic_in_can_approve_contract branch February 12, 2024 11:46
wes-otf pushed a commit that referenced this pull request May 7, 2024
wes-otf pushed a commit that referenced this pull request May 8, 2024
Vldln pushed a commit to equalitie/hypha that referenced this pull request May 28, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Type: Enhancement This is an improvement of an existing thing (not a new thing, which would be a feature). Type: Patch Mini change, used in release drafter
Projects
None yet
Development

Successfully merging this pull request may close these issues.

If a user has staff and contracting roles they can not approve contracts
3 participants