docker-credential-helpers icon indicating copy to clipboard operation
docker-credential-helpers copied to clipboard

pass: base64 encoded username with backward compatility

Open crazy-max opened this issue 2 years ago • 3 comments

carry and closes #267 fixes #162

The allowed characters for usernames in Unix systems typically follow these guidelines:

  • Alphanumeric characters: Both uppercase and lowercase letters (A-Z, a-z) are allowed.
  • Numeric digits: The numbers 0-9 are allowed.
  • Special characters: In most Unix systems, usernames can include the underscore character _. but also - , ..

Looking at shadow utility and the regexp used it seems to match https://github.com/shadow-maint/shadow/blob/dcc90658fd672c63e5498619e77f2d5a3d95f7d7/libmisc/chkname.c#L28-L73

But there are some cases like the credential helper where we can have other special characters to be handled. shadow also needed to allow non-standard usernames. e.g., for compatibility with Samba machine accounts: https://bugs.debian.org/cgi-bin/bugreport.cgi?bug=42874. So it seems ok to rely on base64 encoding for the username.

crazy-max avatar May 28 '23 17:05 crazy-max

Codecov Report

Patch coverage: 100.00% and project coverage change: +0.46 :tada:

Comparison is base (a652f8e) 54.68% compared to head (5fd7864) 55.14%.

Additional details and impacted files
@@            Coverage Diff             @@
##           master     #288      +/-   ##
==========================================
+ Coverage   54.68%   55.14%   +0.46%     
==========================================
  Files           9        9              
  Lines         673      680       +7     
==========================================
+ Hits          368      375       +7     
  Misses        262      262              
  Partials       43       43              
Impacted Files Coverage Δ
pass/pass.go 69.16% <100.00%> (+1.91%) :arrow_up:

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

codecov-commenter avatar May 28 '23 17:05 codecov-commenter

Thanks @crazy-max - I'll try to have a look at this one.

Some things we should consider when merging some of the pending PRs;

  • We're still at v0.x.x
  • Some of the pending PRs are adding new features / drivers
  • ^^ ideally, I think we should use "minor" (e.g. v0.8.0, v0.10.0) updates for these
  • ^^ so that we "reserve" patch (v0.7.1, v0.8.1) updates in case we have bug fixes that we need, without adding new features

So, probably we should have a look at what's already merged, and what's still pending, and then decide what change should go into what "minor" or "patch" release.

I think so far, the changes that were merged since v0.7.0 are all relatively safe (mostly fixes, and the addition of the --version and --help flags); https://github.com/docker/docker-credential-helpers/compare/v0.7.0...83d38ea5e6797b46e277fc31952a4e736fece6d3. To be on the safe side, we could tag those changes as a v0.8.0 (then we'd still have v0.7.x in case we have a problem).

thaJeztah avatar May 30 '23 08:05 thaJeztah