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.

Sign up for GitHub

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

Jump to bottom

Update Content-Type for login request #7253

Merged
merged 6 commits into from
Apr 5, 2024
Merged

Update Content-Type for login request #7253

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
9e20f7b
Update encoding for login
josunect Apr 4, 2024
f06894e
Update login
josunect Apr 4, 2024
efb7407
Add test
josunect Apr 4, 2024
62dee70
Add test
josunect Apr 4, 2024
30a9075
Update test
josunect Apr 4, 2024
51f87f2
Update tests
josunect Apr 4, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Jump to
Jump to file
Failed to load files.
Diff view
Diff view
6 changes: 3 additions & 3 deletions frontend/src/services/Api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,8 +102,8 @@ const loginHeaders = config.login.headers;

/** Helpers to Requests */

const getHeaders = (): Partial<AxiosHeaders> => {
if (apiProxy) {
const getHeaders = (urlEncoded?: boolean): Partial<AxiosHeaders> => {
if (apiProxy || urlEncoded) {
Comment on lines +105 to +106
Copy link
Contributor

Choose a reason for hiding this comment

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

It looks like this leaves off ...loginHeaders. Should those be included for both content types?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I've thought that loginHeaders contain the login information (Once logged in), but let me verify

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updated

Copy link
Contributor

Choose a reason for hiding this comment

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

I'm wondering if OSSMC should also have those login headers? @ferhoyos?

Copy link
Contributor

@ferhoyos ferhoyos Apr 4, 2024 •
edited

Choose a reason for hiding this comment

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

I don't think so, as OSSMC leverages of Openshift login. Where loginHeaders 'X-Auth-Type-Kiali-UI': '1' is used?

Said that, including loginHeaders probably do not affect OSSMC. Maybe you can simplify the code:

    if (apiProxy || urlEncoded) {
      return { 'Content-Type': 'application/x-www-form-urlencoded', ...loginHeaders };
    } else {
      return { 'Content-Type': 'application/json', ...loginHeaders };
   }

return { 'Content-Type': 'application/x-www-form-urlencoded' };
} else {
return { 'Content-Type': 'application/json', ...loginHeaders };
Expand Down Expand Up @@ -143,7 +143,7 @@ export const login = async (
const axiosRequest = {
method: HTTP_VERBS.POST,
url: apiProxy ? `${apiProxy}/${urls.authenticate}` : urls.authenticate,
headers: getHeaders() as AxiosHeaders,
headers: getHeaders(true) as AxiosHeaders,
data: params
};

Expand Down