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

Show file tree
Hide file tree
Changes from all commits
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.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 21 additions & 0 deletions frontend/cypress/integration/common/kiali_login.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -92,3 +92,24 @@ Then('user sees the {string} phrase displayed', (phrase: string) => {
cy.url().should('include', 'login');
}
});

Then('user fills in a valid password', () => {
if (auth_strategy === 'openshift') {
cy.log(`Log in as user with valid password: ${USERNAME}`);

cy.get('#inputUsername').clear().type(`${USERNAME}`);

cy.get('#inputPassword').type(`${PASSWD}`);
cy.get('button[type="submit"]').click();
}
if (auth_strategy === 'token') {
cy.exec('kubectl -n istio-system create token citest').then(result => {
cy.get('#token').type(result.stdout);
cy.get('button[type="submit"]').click();
});
}
});

Then('user sees the Overview page', () => {
cy.get('div[data-test="overview-app-health"]').should('exist');
});
4 changes: 4 additions & 0 deletions frontend/cypress/integration/featureFiles/kiali_login.feature
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -20,3 +20,7 @@ Feature: Kiali login
Scenario: Try to log in with an invalid password
And user fills in an invalid password
Then user sees the "Invalid login or password. Please try again." phrase displayed

Scenario: Try to log in with a valid password
And user fills in a valid password
Then user sees the Overview page
11 changes: 5 additions & 6 deletions frontend/src/services/Api.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -102,12 +102,11 @@ const loginHeaders = config.login.headers;

/** Helpers to Requests */

const getHeaders = (): Partial<AxiosHeaders> => {
if (apiProxy) {
return { 'Content-Type': 'application/x-www-form-urlencoded' };
} else {
return { 'Content-Type': 'application/json', ...loginHeaders };
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
Loading

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', ...loginHeaders };
}
return { 'Content-Type': 'application/json', ...loginHeaders };
};

const basicAuth = (username: UserName, password: Password): BasicAuth => {
Expand Down Expand Up @@ -143,7 +142,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