Skip to content
This repository was archived by the owner on Nov 24, 2025. It is now read-only.

Conversation

@ocket8888
Copy link
Contributor

@ocket8888 ocket8888 commented May 14, 2022

Fixes #6830 and fixes #6831 and fixes #6776 and fixes #6299 and fixes #6396. Note that #6776 is fixed only in APIv4, and remain in earlier versions.

This PR eliminates a separate representation of users used solely for the /user/current endpoint. It makes the endpoint instead use the same representation as /users and /users/{{ID}}. It also fixes some bugs I uncovered in both the API and the docs while editing them for this change. It also removes the field confirmLocalPasswd from users - UIs should make sure that passwords that are input into input[type=password]s are what the user meant to type by asking them to repeat it. That isn't normally checked again by the server, and then also the password is stored twice in the database (but the only valid states for the data are ones where confirmLocalPasswd and localPasswd are degenerate so having both fields is functionally equivalent to having just one). For backwards compatibility, /user/current always updates confirm_local_passwd to the same value as local_passwd whenever the latter changes.

Also it was annoying that changeLogCount was null only in responses from POST requests to /users, and zero in any other similar situation. So I changed it from an *int to an int so that's not possible anymore. It's never actually allowed to be null anyway, it's just ignored in request bodies is all.


Which Traffic Control components are affected by this PR?

  • Documentation
  • Traffic Ops Client (Go)
  • Traffic Ops
  • CDN in a Box (enroller was affected)

What is the best way to verify this PR?

Make sure all existing tests still pass - because of the way the client was implemented, the Go client/API integration tests needed remarkably few changes.

Read the documentation, make sure it's accurate and complete.

Update your user in Traffic Portal. Change your password, and also change something else, to make sure both still work.

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

unknown

PR submission checklist

  • This PR has tests
  • This PR has documentation
  • This PR has a CHANGELOG.md entry
  • This PR DOES NOT FIX A SERIOUS SECURITY VULNERABILITY

@ocket8888 ocket8888 added bug something isn't working as intended Traffic Ops related to Traffic Ops regression bug a bug in existing functionality introduced by a new version medium impact impacts a significant portion of a CDN, or has the potential to do so improvement The functionality exists but it could be improved in some way. authentication Relating to login, registration, passwords, tokens, etc. labels May 14, 2022
@mitchell852 mitchell852 added this to the 7.0.0 milestone May 17, 2022
@ocket8888 ocket8888 force-pushed the to/users-use-same-representation branch from 19a7e84 to bd7f05a Compare May 20, 2022 18:43
Copy link
Member

@shamrickus shamrickus left a comment

Choose a reason for hiding this comment

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

LGTM

  • Representation is now the same
  • Backwards compatibility is maintained
  • Listed bugs are fixed

Just needs conflicts resolved

@ocket8888 ocket8888 force-pushed the to/users-use-same-representation branch from f406f5a to b1a209c Compare May 23, 2022 20:17
@shamrickus shamrickus merged commit d525491 into apache:master May 23, 2022
@ocket8888 ocket8888 deleted the to/users-use-same-representation branch May 23, 2022 21:51
@asf-ci asf-ci mentioned this pull request Jun 1, 2022
4 tasks
@zrhoffman zrhoffman removed the bug something isn't working as intended label Sep 15, 2022
zrhoffman pushed a commit to zrhoffman/trafficcontrol that referenced this pull request Oct 2, 2022
* Convert APIv4 user/current response to use APIv4 /users representation

* Add function to convert ozzo errors to errors and join them in one step

* Make user/current GET requests return UserV40s, remove ConfirmLocalPassword

Also fixed undocumented error where "current user" representations didn't
include GID or UID, and fixed unable to edit UCDN with the user/current
endpoint, and remove the top-level "user" key from v4 that holds the
actual request body, and fix upgrading CurrentUser not setting v4 fields
and fix PUT request responses not showing changeLogCount, GID, UID, and
UCDN.

* Fix ToError never returning a nil error

* Fix unable to update user through APIv4

* Fix APIv4/Go client integration test compilation errors

* Better error handling

* Fix Go APIv4 client using incorrect body structure for PUT /user/current

* Fix `changeLogCount` null in POST responses

* Update documentation

* Update Traffic Portal to account for the new user structure

* Update changelog

* Fix broken footnote linking

* Fix unnecessarily duplicated expression
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.

Labels

authentication Relating to login, registration, passwords, tokens, etc. improvement The functionality exists but it could be improved in some way. medium impact impacts a significant portion of a CDN, or has the potential to do so regression bug a bug in existing functionality introduced by a new version Traffic Ops related to Traffic Ops

Projects

None yet

4 participants