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.

Sign up for GitHub

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

Jump to bottom

VO members can re-sign the AUP at any time #757

Merged
merged 25 commits into from
May 29, 2024
Merged

VO members can re-sign the AUP at any time #757

merged 25 commits into from
May 29, 2024

Conversation

garaimanoj
Copy link
Contributor

@garaimanoj garaimanoj commented Apr 24, 2024 •
edited by enricovianello

Fixes #736

@garaimanoj @enricovianello
Add ability to disable client
9451f09 
* Add API and service methods to change client status
* Add new columns for client status
* Set status as active for new client
* Add suspended label next to client name
* Add column status_changed_by to client_details
* Make client suspension details available to client owner
@garaimanoj
Make fields available for limited client view
95a8c19
@garaimanoj
Throw exception on update of suspended client
ba51133
@garaimanoj
Handle client suspended exception
5062702
@garaimanoj
Add AUP re-sign button
4a62227
@garaimanoj
Add AUP re-sign modal and controller
a9ea662
@garaimanoj
Add sign AUP function
615d548
@garaimanoj garaimanoj self-assigned this Apr 24, 2024
@garaimanoj garaimanoj linked an issue Apr 24, 2024 that may be closed by this pull request
VO members should be able to re-sign the AUP at any time #736
Closed
@garaimanoj
Copy link
Contributor Author

image
image

@garaimanoj garaimanoj marked this pull request as ready for review April 26, 2024 14:46
@garaimanoj
Copy link
Contributor Author

Two points to be considered

  • Do we need to change the message in the re-sign page. Currently it is In order to proceed, you need to sign ..........
  • Currently we are not capturing the time zone when storing the signature time in database. Will it be a issue in future if IAM is deployed in High Availability mode.

@enricovianello enricovianello changed the title Issue 736 re sign aup VO members can re-sign the AUP at any time May 2, 2024
enricovianello
enricovianello requested changes May 2, 2024
View reviewed changes
Copy link
Member

@enricovianello enricovianello left a comment

Choose a reason for hiding this comment

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

I found that the PATCH endpoint you added already exists through another API (it needs a little fix). Probably with less changes it works too!

iam-login-service/src/main/java/it/infn/mw/iam/api/aup/AupSignaturePageController.java Outdated Show resolved Hide resolved
iam-login-service/src/main/java/it/infn/mw/iam/api/aup/AupSignaturePageController.java Outdated Show resolved Hide resolved
iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/aup.service.js Outdated Show resolved Hide resolved
enricovianello
enricovianello requested changes May 15, 2024
View reviewed changes
Copy link
Member

@enricovianello enricovianello left a comment

Choose a reason for hiding this comment

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

The main comments are: there's no really a need to add an AupResignedEvent (the AupSignedEvent can be used) and probably there's also no need to add endpoint and users can sign again by using a POST on '/iam/aup/signature'

iam-login-service/src/main/java/it/infn/mw/iam/audit/events/aup/AupResignedEvent.java Outdated Show resolved Hide resolved
...service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/user.component.html Outdated Show resolved Hide resolved
...n-service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/user.component.js Outdated Show resolved Hide resolved
iam-login-service/src/main/webapp/resources/iam/apps/dashboard-app/services/aup.service.js Outdated Show resolved Hide resolved
iam-login-service/src/main/java/it/infn/mw/iam/api/aup/AupSignaturePageController.java Outdated Show resolved Hide resolved
enricovianello
enricovianello requested changes May 23, 2024
View reviewed changes
Copy link
Member

@enricovianello enricovianello left a comment

Choose a reason for hiding this comment

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

Probably we need to hide the button in case of admin is viewing a user page

...service/src/main/webapp/resources/iam/apps/dashboard-app/components/user/user.component.html Outdated
@@ -66,7 +66,7 @@ <h1>

<!-- Send reset password email or change password-->
<user-password user="$ctrl.user"></user-password>

<aup-resign user="$ctrl.user"></aup-resign>
Copy link
Member

Choose a reason for hiding this comment

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

This object is visible to both admins and users but admins can enter user's page and will find this button that acts for theirselves and not for the user. This object should probably be hidden in case of !$ctrl.isMe() && $ctrl.isVoAdmin(). Could you check this behavior and confirm this?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yes, I have checked if admin enter user's page admin can see the button.
If I add ng-if="$ctrl.isMe()" in aup-resign tag then it is working as expected.

@enricovianello enricovianello changed the base branch from issue-705 to develop May 24, 2024 13:32
@enricovianello
Copy link
Member

I changed base branch to develop, having merged #705 into develop branch

@sonarcloud SonarCloud
Copy link

sonarcloud bot commented May 24, 2024

Quality Gate Passed Quality Gate passed

Issues
0 New issues
0 Accepted issues

Measures
0 Security Hotspots
No data about Coverage
0.0% Duplication on New Code

See analysis details on SonarCloud

@enricovianello enricovianello merged commit eac5eb3 into develop May 29, 2024
7 checks passed
@garaimanoj garaimanoj deleted the issue-736-re-sign-AUP branch June 10, 2024 13:44
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@enricovianello enricovianello enricovianello approved these changes

@federicaagostini federicaagostini Awaiting requested review from federicaagostini

Assignees

@garaimanoj garaimanoj

Labels
None yet
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

VO members should be able to re-sign the AUP at any time
2 participants
@garaimanoj @enricovianello