This repository was archived by the owner on Nov 24, 2025. It is now read-only.
-
Notifications
You must be signed in to change notification settings - Fork 351
Make users use same representation across endpoints #6832
Merged
shamrickus
merged 14 commits into
apache:master
from
ocket8888:to/users-use-same-representation
May 23, 2022
Merged
Make users use same representation across endpoints #6832
shamrickus
merged 14 commits into
apache:master
from
ocket8888:to/users-use-same-representation
May 23, 2022
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
shamrickus
suggested changes
May 20, 2022
19a7e84 to
bd7f05a
Compare
shamrickus
approved these changes
May 23, 2022
Member
shamrickus
left a comment
There was a problem hiding this 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
…ssword 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.
f406f5a to
b1a209c
Compare
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
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
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/currentendpoint. It makes the endpoint instead use the same representation as/usersand/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 fieldconfirmLocalPasswdfrom users - UIs should make sure that passwords that are input intoinput[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 whereconfirmLocalPasswdandlocalPasswdare degenerate so having both fields is functionally equivalent to having just one). For backwards compatibility,/user/currentalways updatesconfirm_local_passwdto the same value aslocal_passwdwhenever the latter changes.Also it was annoying that
changeLogCountwasnullonly in responses from POST requests to/users, and zero in any other similar situation. So I changed it from an*intto anintso that's not possible anymore. It's never actually allowed to benullanyway, it's just ignored in request bodies is all.Which Traffic Control components are affected by this PR?
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