TO: Only display Server ILO/XMPP Passwords Based on New Permission by shamrickus · Pull Request #7697 · apache/trafficcontrol · GitHub
Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

TO: Only display Server ILO/XMPP Passwords Based on New Permission#7697

Merged
zrhoffman merged 3 commits into
apache:masterfrom
shamrickus:to/server-priv
Aug 8, 2023
Merged

TO: Only display Server ILO/XMPP Passwords Based on New Permission#7697
zrhoffman merged 3 commits into
apache:masterfrom
shamrickus:to/server-priv

Conversation

@shamrickus

@shamrickus shamrickus commented Aug 3, 2023

Copy link
Copy Markdown
Member

Currently, the servers endpoints will display the xmppPasswd/iloPassword based only on privlevel. In api v5/v4 this has been changed to check for a new permission SECURE-SERVER:READ instead (if enabled).


Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Ops

What is the best way to verify this PR?

Confirm the TO Tests work. For version api version 3/4 and role_based_permissions is off confirm that Ops/Admin PrivLevel is required to view the two password fields. For Version 4, confirm that with role_based_permissions set, that only users that are admin or have the SECURE-SERVER:READ permission can view the password fields. For version 5, it should only check for the permission.

If this is a bugfix, which Traffic Control versions contained the bug?

  • master
  • 7.0.1

PR submission checklist

@shamrickus shamrickus added bug something isn't working as intended Traffic Ops related to Traffic Ops low impact affects only a small portion of a CDN, and cannot itself break one permissions Dealing with roles/permissions/tenancy labels Aug 3, 2023
@codecov

codecov Bot commented Aug 3, 2023

Copy link
Copy Markdown

@mitchell852

Copy link
Copy Markdown
Member

@shamrickus can you also create the SECURE-SERVER:READ permission via a database migration?

@mitchell852

mitchell852 commented Aug 7, 2023

Copy link
Copy Markdown
Member

@shamrickus is there a reason this can't be "fixed" in api v3 and v4? i know we don't like to change behavior of a published api but fixing a bug seems like it would be fair game.

@shamrickus

Copy link
Copy Markdown
Member Author

@shamrickus is there a reason this can't be "fixed" in api v3 and v4? i know we don't like to change behavior of an api but fixing a bug seems like it would be fair game.

API v3 doesn't have the new role based permissions and this fixes it in v4.

@mitchell852

Copy link
Copy Markdown
Member

@kdamichie kdamichie left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

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

Code looks good, and tested functionality

@zrhoffman zrhoffman merged commit 849d166 into apache:master Aug 8, 2023
cybertunnel pushed a commit to cybertunnel/trafficcontrol that referenced this pull request Aug 16, 2023
…pache#7697)

* Remove priv checks for secure server fields

* Handle api version 4 also

* Forgot a file + changelog

---------

Co-authored-by: Steve Hamrick <shamrick@apache.org>
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

bug something isn't working as intended low impact affects only a small portion of a CDN, and cannot itself break one permissions Dealing with roles/permissions/tenancy Traffic Ops related to Traffic Ops

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants