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.

Sign up for GitHub

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

Jump to bottom

docs(auth): clarify "Bearer token" terminology #11714

Merged
merged 3 commits into from
Oct 18, 2023
Merged

docs(auth): clarify "Bearer token" terminology #11714

merged 3 commits into from
Oct 18, 2023

Conversation

agilgur5
Copy link
Member

@agilgur5 agilgur5 commented Aug 30, 2023 •
edited
Loading

Follow-up to #11300, where my wording could have been clearer

Motivation

Per #11680 and some others, this is still a point of confusion with users

Modifications

  • be specific when referring to $ARGO_TOKEN, which includes "Bearer ", and the SA Secret, which is the part after "Bearer "
  • also fix the one doc that has Bearer $ARGO_TOKEN whereas the rest of the codebase has $ARGO_TOKEN with "Bearer " part of it

Verification

make docs passes

Related Notes

  • Note: the internal terminology is actually a bit off here. In the IETF standards spec, the "token" is referred to as the part after "Bearer "
    • The spec does not quite specify this in its terminology section, however, which means this is not as clear as it could be in the industry
    • See also another common industry reference's use of the terminology: https://swagger.io/docs/specification/authentication/bearer-authentication/
    • Internally though (which is more than just docs, it includes source code and tests as well), $ARGO_TOKEN always includes "Bearer "
      • so this change makes everything internally consistent. changing all internals would be very complex at this time

@agilgur5
docs(auth): clarify "Bearer token" terminology
16bd3c0 
- follow-up to 50395d2, where my wording could have been clearer

- this is still a point of confusion with users, so be specific when referring to `$ARGO_TOKEN`, which includes "Bearer ", and the SA Secret, which is the part after "Bearer "
  - also fix the one doc that has `Bearer $ARGO_TOKEN` whereas the rest of the codebase has `$ARGO_TOKEN` with "Bearer " part of it

- **Note**: the _internal_ terminology is actually a bit off here. In the [IETF standards spec](https://datatracker.ietf.org/doc/html/rfc6750#section-2.1), the "token" is referred to as the part after "Bearer "
  - The spec does not quite specify this in its [terminology section](https://datatracker.ietf.org/doc/html/rfc6750#section-1.2) though
  - Internally (which is more than just docs, it includes source code and tests as well) though, `$ARGO_TOKEN` _always_ includes "Bearer "
  - See also another common industry reference's use of the terminology: https://swagger.io/docs/specification/authentication/bearer-authentication/

Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
@agilgur5 agilgur5 added the area/docs Incorrect, missing, or mistakes in docs label Aug 30, 2023
@agilgur5 agilgur5 mentioned this pull request Oct 16, 2023
Merged
@agilgur5
Copy link
Member Author

Bearer was removed in #12014, which was reviewed and merged before this PR, despite being made ~1.5 months after it... 😕
That PR did not have any description, but this one does describe in detail that Bearer is currently included in the $ARGO_TOKEN variable in the codebase, but probably shouldn't be according to industry terminology.

I merged master as a result, but this description and remaining changes are still relevant

@terrytangyuan terrytangyuan enabled auto-merge (squash) October 16, 2023 19:51
@agilgur5
Copy link
Member Author

agilgur5 commented Oct 18, 2023 •
edited
Loading

Was failing on a flakey test, so I merged master (again) to get #12006 in, which will no longer run tests for docs-only changes

@terrytangyuan terrytangyuan merged commit 5c264c0 into argoproj:master Oct 18, 2023
14 of 15 checks passed
@agilgur5 agilgur5 deleted the docs-token-term-clarify branch October 18, 2023 21:00
agilgur5 added a commit that referenced this pull request May 4, 2024
@agilgur5
docs(auth): clarify "Bearer token" terminology (#11714)
7e71471 
Signed-off-by: Anton Gilgur <agilgur5@gmail.com>
(cherry picked from commit 5c264c0)
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@jmeridth jmeridth jmeridth approved these changes

@terrytangyuan terrytangyuan terrytangyuan approved these changes

Assignees
No one assigned
Labels
area/docs Incorrect, missing, or mistakes in docs
Projects
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
3 participants
@agilgur5 @jmeridth @terrytangyuan