fix: undo breaking Authorization header change while allowing Bearer auth to be configured

[alombardo4]

Hey, I just made a Pull Request!

This PR removes the "breakingness" of a breaking change in the sonarqube-backend plugin introduced in v0.7.0 via a breaking change introduced in https://github.com/backstage/community-plugins/pull/3469.

This PR makes Basic auth once again the default, and adds a new optional configuration parameter tokenKind. It is an optional string, and when set to either bearer or Bearer will enable Bearer tokens to be used. Any other value (including undefined) for tokenKind should result in reverting to the previous behavior of Basic auth.

The 0.7.0 change broke our working setup with self-hosted Sonarqube Enterprise Edition v9.9.9.

:heavy_check_mark: Checklist

• [x] A changeset describing the change and affected packages. (more info)
• [x] Added or updated documentation
• [x] Tests for new functionality and regression tests for bug fixes
• [ ] Screenshots attached (for UI changes)
• [x] All your commits have a Signed-off-by line in the message. (more info)

[backstage-goalie[bot]]

Changed Packages

Package Name	Package Path	Changeset Bump	Current Version
@backstage-community/plugin-sonarqube-backend	workspaces/sonarqube/plugins/sonarqube-backend	patch	v0.9.0

[awanlin]

HI @alombardo4, sorry the last release of the plugin broke things for you. Before we go further I have a umber of questions to help me better understand where we, most me, went wrong with the previous PR.

The previous PR has links to docs clearly showing that this should work which is why we moved forward and why I didn't ask for it to be marked as explicitly breaking. Can you share any docs as to why this didn't work for you? Can you share details about errors or the experience you had?

Also, can you help me understand the SonarQube server version policy? You are on 9.9 which the docs have tagged as LTA, I'm having some trouble understanding what that means?

@RedlineTriad, can you help answer these questions as well?

[alombardo4]

Hi @awanlin thanks for taking a look so quickly.

In v9.9.9, the docs say it supports Basic auth here: https://docs.sonarsource.com/sonarqube-server/9.9/extension-guide/web-api/

This seems to have changed in newer versions. Sonarqube LTA (https://www.sonarsource.com/products/sonarqube/downloads/lts/) is their concept of long term support. Right now, 9.9.9 is the oldest version still supported, but newer versions should support both Basic and Bearer auth (with the exception of Sonarcloud as I understand it).

This break manifested itself as a 401 when calls were made from the Backstage backend to our Sonarqube server. We pinpointed the day that the integration started throwing 401s as the day we upgraded to v0.7.0, then managed to reproduce the issue on a fresh Backstage codebase with just this plugin installed. Reverting to v0.6.0 resolved the issue both locally and in our running instance.

Organizationally, the team I am working with on Backstage does not have the ability or access to choose to upgrade Sonarqube. We do need to maintain some support for Basic auth to keep v9.9.9 working, but if you'd prefer, I can update this PR to make Bearer the new default and maintain the behavior of v0.7.0 while enabling us (and others stuck on v9.9.9 or older) to use Basic auth by providing additional configuration.

[awanlin]

Hi @alombardo4, thanks for these details, that really helps. I don't think it's reasonable for us to say go upgrade especially given this worked with the previous version. I do think we should say we support LTA versions. That lines up conceptually with how we support LTS versions of Node for Backstage itself.

Now for how we fix this: I think we should stick with your proposed solution with the tokenKind makes sense. I won't have time to review this in detail today though. Top of my list for tomorrow!

[alombardo4]

@awanlin Thanks for the review. I've updated the PR with all your suggestions.

[alombardo4]

@vinzscam @awanlin Is anything else needed from me on this?

[awanlin]

HI @mario-mui @andrewthauer @vinzscam, if there are no further truly blocking issue with this PR I plan to merge this tomorrow morning CST. We really need to ship this fix to prevent more people from running into the same issue. 👍

[andrewthauer]

Ugh, serves me for not searching for existing PRs before I created my own. Yep, I'm fine using this PR though.

[lavanya-sainik-ericsson]

Hello All,

We also need this PR. ( As already mentioned that upgrading to higher version is broken right now). Would be great to get the fix merged if no more concerns/ comments.

[andrewthauer]

I completely overlooked this, but I believe this if condition is inverted which likely breaks both authTypes now https://github.com/backstage/community-plugins/pull/3870/files#diff-e20ad5f49a5b543f22abe9741fa330ed4615b77004f8af35a8b2217bf91701d8R325.

[lavanya-sainik-ericsson]

Hello @alombardo4 , @awanlin, @vinzscam

@andrewthauer yes looks like you are right. I tried upgrading to 0.9.1 version and again faced same problem. After some troubleshooting looks like this condition seems wrong

const encodedAuthToken = authType === 'Bearer' ? Buffer.from(${authToken}:).toString('base64') : authToken;

It should be 'Basic' I think . Please share your views.

[alombardo4]

Hello @alombardo4 , @awanlin, @vinzscam

@andrewthauer yes looks like you are right. I tried upgrading to 0.9.1 version and again faced same problem. After some troubleshooting looks like this condition seems wrong

const encodedAuthToken = authType === 'Bearer' ? Buffer.from(${authToken}:).toString('base64') : authToken;

It should be 'Basic' I think . Please share your views.

Yes, I think that should have been Basic. It seems to have gotten mixed up in some of the back and forth. I'll raise another PR shortly.

[awanlin]

Hi @alombardo4 @lavanya-sainik-ericsson @andrewthauer, can you coordinate to submit a PR fix or at least log an issue for this so it doesn't get lost in the day to day? Sorry, once a PR gets merged it's really easy to miss the comments.

[alombardo4]

@awanlin I just raised https://github.com/backstage/community-plugins/pull/4257 to reverse that if statement

[MetallFoX]

You made my day, thanx!