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

Fixes #12791: Have a reset-keys button on the node screen #5547

Conversation

fanf
Copy link
Member

@fanf fanf commented Mar 27, 2024

https://issues.rudder.io/issues/12791

Add a button to reset the key status if:

  • current user has write admin rights,
  • current key status is "certified".

On backend, it's just a call to the relevant woNodeRepository function, plus some html/ajax to make it works.
I needed to extract the div with security info to make it displayable again after change.
I prefered to rebuild it all than just changing the icon because the key/key status logic is complex and I wanted to use the same logic for displaying (so that nothing change after reload).

change-key-status.webm

@fanf fanf requested a review from VinceMacBuche March 27, 2024 21:19
* Display the div with ID `security-info` with cfenfine and curl key/certificate hash,
* the key/certificate detail, and the button to untrust the key
*/
private def displaySecutiryInfo(node: NodeInventory): NodeSeq = {
Copy link
Member

Choose a reason for hiding this comment

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

Secutiry ;)

@amousset
Copy link
Member

I'd prefer the button to be visible and not require to unfold the key (and we'll need to do it for 8.1 anyway)

@VinceMacBuche
Copy link
Member

Why Administration rights ? Administration is to manage rudder server and here to manage a Node parameter. I know that's a sensitive operation and can understand the will to put it behind walls, but I would have set "Node write"

The api to update a Node, and reset it's key status is behind Node Edit right, (maybe not a good choice, like all edit) but i would have put the same right

@fanf
Copy link
Member Author

fanf commented Mar 28, 2024

PR updated with a new commit

@fanf
Copy link
Member Author

fanf commented Mar 28, 2024

change-key-status-2.webm

@fanf
Copy link
Member Author

fanf commented Mar 28, 2024

PR updated with a new commit

1 similar comment
@fanf
Copy link
Member Author

fanf commented Mar 28, 2024

PR updated with a new commit

@Normation-Quality-Assistant
Copy link
Contributor

This PR is not mergeable to upper versions.
Since it is "Ready for merge" you must merge it by yourself using the following command:
rudder-dev merge https://github.com/Normation/rudder/pull/5547
-- Your faithful QA
Kant merge: "Happiness is not an ideal of reason, but of imagination."
(https://ci.normation.com/jenkins/job/merge-accepted-pr/83105/console)

@fanf
Copy link
Member Author

fanf commented Apr 17, 2024

OK, squash merging this PR

@fanf fanf force-pushed the enh_12791/have_a_reset_keys_button_on_the_node_screen branch from afe2fc7 to f739b3e Compare April 17, 2024 15:23
@fanf fanf merged commit f739b3e into Normation:branches/rudder/7.3 Apr 17, 2024
0 of 6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
4 participants