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

Enable OIDC redirect in dashboard #3233

Merged
merged 6 commits into from
Feb 25, 2023
Merged

Enable OIDC redirect in dashboard #3233

Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
31de196
Enable OIDC redirect in dashboard
npalaska Feb 6, 2023
db6499c
update the buttons on UI
npalaska Feb 7, 2023
00d41a9
add try-catch, sync with latest server endpoints api changes
npalaska Feb 8, 2023
d32e20b
we dont need client secret when redirecting
npalaska Feb 8, 2023
de04230
refactor and remove redux state mutation
npalaska Feb 11, 2023
852762e
Merge branch 'openid-connect' into pbench_1072
npalaska Feb 23, 2023
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
47 changes: 21 additions & 26 deletions dashboard/src/actions/authActions.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -13,39 +13,34 @@ import { uid } from "../utils/helper";
export const authenticationRequest = () => async (dispatch, getState) => {
try {
const endpoints = getState().apiEndpoint.endpoints;
const oidcServer = endpoints["openid-connect"]?.issuer;
const oidcRealm = endpoints["openid-connect"]?.realm;
const oidcClient = endpoints["openid-connect"]?.client;
const oidcServer = endpoints.openid.server;
const oidcRealm = endpoints.openid.realm;
const oidcClient = endpoints.openid.client;
// URI parameters ref: https://openid.net/specs/openid-connect-core-1_0.html#AuthorizationEndpoint
// Refer Step 3 of pbench/docs/user_authentication/third_party_token_management.md
let req = oidcServer + '/realms/' + oidcRealm + '/protocol/openid-connect/auth';
req += '?client_id=' + oidcClient;
req += '&response_type=code';
req += '&redirect_uri=' + window.location.href.split('?')[0];
req += '&scope=profile';
req += '&prompt=login';
req += '&max_age=120';
window.location.href = req;
const uri = `${oidcServer}/realms/${oidcRealm}/protocol/openid-connect/auth`;
const queryParams = [
'client_id=' + oidcClient,
'response_type=code',
'redirect_uri=' + window.location.href.split('?')[0],
'scope=profile',
'prompt=login',
'max_age=120'
];
window.location.href = uri + '?' + queryParams.join('&');
} catch (error) {
const alerts = getState().userAuth.alerts;
let alert = {};
if (error?.response) {
alert = {
title: error?.response?.data?.message,
key: uid(),
};
dispatch(toggleLoginBtn(true));
} else {
alert = {
title: error?.message,
dispatch(error?.response
? toggleLoginBtn(true)
: { type: TYPES.OPENID_ERROR }
);
const alert = {
title: error?.response ? error.response.data?.message : error?.message,
Copy link
Member

@webbnh webbnh Feb 13, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

This feels funny: it says "if there is an error.response then there must be an error.response.data which might contain a message which we can use for the title (but, if it doesn't, then use a title of undefined); otherwise, use a title of error.message (or maybe undefined). That's a lot of undefined's....

In any case, it seems like what we want is something more like this:

          title: error.response?.data?.message
            ? error.response.data.message
            : error?.message ? error.message : "Unexpected error",

That is, if our error.response contains a data which contains a message, then use that message; otherwise, if our error contains a message then use that message; otherwise, fall back to a constant message.

(The current code is assuming that error.response contains a data which may or may not contain a message...are we sure it contains a data?...are we sure that data might not contain a message?)

Copy link
Member Author

Choose a reason for hiding this comment

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

Okay, this is a good suggestion but I will address it in #3250, I need to do some rework there anyway.

key: uid(),
};
dispatch({ type: TYPES.OPENID_ERROR });
}
alerts.push(alert);
};
dispatch({
type: TYPES.USER_NOTION_ALERTS,
payload: alerts,
payload: [...alerts, alert],
});
dispatch({ type: TYPES.COMPLETED });
}
Expand Down