docker / docker-credential-helpers

Programs to keep Docker login credentials safe by storing in platform keystores
MIT License
1.05k stars 166 forks source link

pass: base64 encoded username with backward compatility #288

Open crazy-max opened 1 year ago

crazy-max commented 1 year ago

carry and closes #267 fixes #162

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

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.

codecov-commenter commented 1 year ago

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 ```diff @@ 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](https://app.codecov.io/gh/docker/docker-credential-helpers/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=docker) | Coverage Δ | | |---|---|---| | [pass/pass.go](https://app.codecov.io/gh/docker/docker-credential-helpers/pull/288?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=docker#diff-cGFzcy9wYXNzLmdv) | `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.

thaJeztah commented 1 year ago

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;

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).