Arvados

Ok, so mostly this is me being stupid because I hadn't realised that I was supposed to send my arvados username and api token rather than my actual SSO username/password to arvados-git-httpd.

After reading Tom's updated documentation in issue 6663, that suddenly became clear to me.

Leaking Arvados API tokens to the log is probably not so bad, but I suspect that log will end up accidentlaly containing a lot of actual user passwords (because git does prompt for 'password').

Perhaps you could check the provided API token to verify that it appears to be in the shape of an API token, and if so it could be printed.. otherwise assume it might be some other password and omit it? I suspect not very many end users will have 50 character passwords...

Joshua Randall wrote:

Perhaps you could check the provided API token to verify that it appears to be in the shape of an API token, and if so it could be printed.. otherwise assume it might be some other password and omit it? I suspect not very many end users will have 50 character passwords...

Perhaps we should log only the first few characters of the token if we get as far as getting an API response and the API server response indicates it's a valid token, otherwise the string "<invalid>"?

(The first few characters should be enough to cross-reference for troubleshooting purposes, without being enough to cause any trouble if the log file is leaked.)

3959d7afff8bb3c3b8da9eb7d178919275180f2a

• Log only when it is a valid token
• And, even then, only log the first 10 characters of that valid token
• Also, added a two more tests. One with an invalid / non-existing token, and another with an expired token.

I'm reviewing 3959d7afff8bb3c3b8da9eb7d178919275180f2a

After reading the issue. It's unclear to me from the code that is the token being truncated to the first 10 characters.

Probably renaming "password" or adding a comment to that section of the code will make it more clear (and avoid future confusion)

Besides that the branch looks good to me .

Thanks Nico. Added a comment in auth_handler.go as suggested.