[Bitbucket] Rework required

[consideRatio]

#676 reported an issue, but it untangled to multiple issues that needed to be resolved so I've now opened this issue to summarize the findings. I think a proper rework of BitBucketOAuthenticator is required.

• allowed_teams should be deprecated and renamed allowed_workspaces
• (security) allowed_teams works with workspace's name fields (display name) that isn't unique, not their uuid or slug (workspace id) fields
  The functionality of checking workspace membership is currently broken though
• When constructing an Authorization headers for web requests we must send requests with Bearer and it mustn't be bearer as it is if we reference a previous response's token_type value -> [All] Correcting Bearer Authorization header #698
• When using BitBuchetOAuthenticator, the default scopes isn't sufficient I think.
  I know we make requests to endpoints that at least requires the account scope, but perhaps not email.

[jyio]

I recently ran into the Authorization: Bearer issue while trying to integrate GenericOAuthenticator with Kanidm, and we'd likely hit the same snag with any OAuth2 IdP that doesn't follow Postel's law.

RFC 6749 says token_type is case-insensitive in the access token response. RFC 6750 says Bearer should be capitalized in the Authorization request header. This has caused many headaches related to inconsistent case usage. Bitbucket and Kanidm are technically RFC-compliant by sending bearer and expecting Bearer; but I believe it would be more consistent and just as compliant to send Bearer. On the other hand, OAuthenticator is non-compliant by receiving bearer and sending bearer; and the fix is likely simple:

Option 1: Hard-code Bearer, as recommended by this user and you. It would work for most cases, but miss the unusual IdP that wants to use another scheme such as Basic, Digest, or HOBA.

Option 2: Title-case token_type as token_type.lower().title(), which is how I fixed the Kanidm integration. It would work for Basic, Digest, and Bearer schemes but mess up HOBA. If we wanted to do this, we might need to special-case HOBA.

Option 3: Rewrite any case of bearer to Bearer; e.g. 'Bearer' if token_type.lower() == 'bearer' else token_type. This would selectively fix the common case of Bearer authentication while keeping the current behavior of mirroring the IdP if the IdP wanted to try something else. Probably the most sensible approach.

Option 4: Option 3, but also for Basic, Digest, HOBA, etc... falling back to current behavior if an unknown scheme is used. A dictionary could help, e.g. {x.lower(): x for x in ('Basic', 'Digest', 'Bearer', 'HOBA')}[token_type.lower()]. Most comprehensive, but probably overkill.

Also, which file should be patched? Even though I use GenericOAuthenticator, the error was raised in oauth2.py so that's the file I edited. We'd probably have to factor out this auth-scheme normalizing code so it could be shared by multiple authenticators...

[consideRatio]

@jyio this was a fantastic summary!! I agree os option Option 3 as a sensible approach, and that was what @yaleman did in #698 referencing back here - thank you both!! ❤️ 🎉 🌻

[consideRatio]

Also, which file should be patched? Even though I use GenericOAuthenticator, the error was raised in oauth2.py so that's the file I edited. We'd probably have to factor out this auth-scheme normalizing code so it could be shared by multiple authenticators...

The things in oauth2.py is defining the OAuthenticator base class common for all authenticators, so updating it there was perfect - at least now that this thorough research establishes it should be the right call for all authenticators!