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

Display secure parameters when role has proper perms #7688

Merged
merged 6 commits into from
Aug 9, 2023

Conversation

ericholguin
Copy link
Contributor

Currently when role based permissions are enabled and a user has the PARAMETERS-SECURE:READ permissions they are still unable to view a secure parameter's value.

This PR fixes that and user's can view the secure value when they have permission.


Which Traffic Control components are affected by this PR?

  • Traffic Ops

What is the best way to verify this PR?

Run Traffic Ops with role_based_permissions enabled

Will need to have a non admin role with PARAMETERS-SECURE:READ permissions and a Parameter with secure set to true

Should be able to view the value with the non-admin role when using 4.0 and should see hidden value when using 3.0 and value should be hidden when the role does not have the PARAMETERS-SECURE:READ permission

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

I'm not sure?

PR submission checklist

@ericholguin ericholguin 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 1, 2023
@codecov
Copy link

codecov bot commented Aug 1, 2023

Codecov Report

Merging #7688 (a26618c) into master (849d166) will decrease coverage by 0.01%.
The diff coverage is 0.00%.

@@             Coverage Diff              @@
##             master    #7688      +/-   ##
============================================
- Coverage     29.78%   29.78%   -0.01%     
  Complexity       98       98              
============================================
  Files           802      802              
  Lines         85789    85797       +8     
  Branches        952      952              
============================================
+ Hits          25550    25551       +1     
- Misses        58099    58107       +8     
+ Partials       2140     2139       -1     
Flag Coverage Δ
golib_unit 48.04% <ø> (ø)
grove_unit 4.60% <ø> (ø)
t3c_unit 4.95% <ø> (ø)
traffic_monitor_unit 21.30% <ø> (ø)
traffic_ops_integration 69.39% <ø> (ø)
traffic_ops_unit 22.27% <0.00%> (-0.01%) ⬇️
traffic_portal_v2 73.71% <ø> (+0.01%) ⬆️
traffic_stats_unit 10.14% <ø> (ø)
unit_tests 26.93% <0.00%> (-0.01%) ⬇️
v3 57.79% <ø> (ø)
v4 79.18% <ø> (ø)
v5 78.52% <ø> (ø)

Flags with carried forward coverage won't be shown. Click here to find out more.

Files Changed Coverage Δ
...fic_ops/traffic_ops_golang/parameter/parameters.go 26.70% <0.00%> (-1.40%) ⬇️

... and 1 file with indirect coverage changes

📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more

@ericholguin ericholguin requested a review from ocket8888 August 7, 2023 16:21
Copy link
Contributor

@ocket8888 ocket8888 left a comment

Choose a reason for hiding this comment

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

LGTM

@ericholguin ericholguin merged commit 16cffe1 into apache:master Aug 9, 2023
@ericholguin ericholguin deleted the secure-params-fix branch August 9, 2023 16:45
cybertunnel pushed a commit to cybertunnel/trafficcontrol that referenced this pull request Aug 16, 2023
* fix conditional for secure parameters

* add change

* handle v5 functionality
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
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.

2 participants