Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Make repos' bearer auth prefix-safe #5097

Merged
merged 4 commits into from
Jul 26, 2022

Conversation

antgamdia
Copy link
Contributor

@antgamdia antgamdia commented Jul 19, 2022

Description of the change

This PR addresses the Bearer authentication in the two plugins supporting it (Helm and Carvel) and makes the implementation "prefix-safe". Regardless of what the API client send (either Bearer foo or foo) the prefix will be trimmed out and the secret's content will become Bearer foo.

Benefits

The UI will be able to send the bearer token regardless of the plugin's implementation.

Possible drawbacks

N/A

Applicable issues

Additional information

I haven't changed anything related to the terminology as, in supporting both (prefixed/non-prefixed) the "token" can actually be the token and not the whole header. Anyway, happy to rename if you still think so.

Edit: squeezing some golang-ci-lint-related code; the tool (used in the gh action) got updated and new checks were being triggered. Ignoring them as it is just code used for testing.

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
@netlify
Copy link

netlify bot commented Jul 19, 2022

Deploy Preview for kubeapps-dev canceled.

Built without sensitive environment variables

Name Link
🔨 Latest commit c43f7d0
🔍 Latest deploy log https://app.netlify.com/sites/kubeapps-dev/deploys/62d6be6f834e0900080e0180

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
@@ -524,8 +524,7 @@ func (s *Server) GetInstalledPackageDetail(ctx context.Context, request *corev1.
}
}
// trim the new doc separator in the last element
//nolint:staticcheck
valuesApplied = strings.Trim(valuesApplied, "---")
valuesApplied = strings.TrimSuffix(valuesApplied, "---")
Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The golint gh action said we were using .Trim incorrectly: it trims chars (unicode runes), so using --- was -partially- wrong, as it wasn't removing the occurrences of the string --- but rather removing every occurrence of -.

To trim strings, we should use TrimSuffix or TrimPrefix instead (or Replace even). See https://topic.alibabacloud.com/a/the-difference-between-the-trimright-and-trimsuffix-of-the-golang-strings-package_1_38_30927804.html

Squeezing this change in this PR.

Signed-off-by: Antonio Gamez Diaz <agamez@vmware.com>
Copy link
Collaborator

@castelblanque castelblanque left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM

@antgamdia antgamdia merged commit 43c0912 into vmware-tanzu:main Jul 26, 2022
@antgamdia antgamdia deleted the 5028-inconsistentBearerAuth branch July 26, 2022 10:07
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

[Package repositories] Review and fix backend and frontend handling of auth headers
3 participants