Add back token logic to invite users endpoints #1643
Merged
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
florkbr
commented
Aug 8, 2024
| @@ -55,9 +56,10 @@ function handleError(error, reject) { | |||
| } | |||
|
|
|||
| export async function addUsers(usersData = { emails: [], isAdmin: undefined }) { | |||
| const token = await insights.chrome.auth.getToken(); | |||
There was a problem hiding this comment.
FWIW we should not be using the insights global. This was originally how the logic was migrated over from RHEcosystem in #1635. We can update the reducers/actions to accept a token from the useAuth hook in the future.
InsaneZein
approved these changes
Aug 8, 2024
catastrophe-brandon
approved these changes
Aug 8, 2024
florkbr
force-pushed
the
fix-invite-users-no-auth-error
branch
from
ccc4a2c to
d086e1a
Compare
|
Pipeline passing now due to #1652. Tested locally and everything is working as expected. I'm going to merge this to unblock inboundary and will keep an eye out for any issues. |
9 tasks
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
We recently migrated the changes from RHEcosystem rbac-ui into this repo. With those changes I made some tweaks per code review comments. One of those tweaks was to remove the token logic (as we suspected chrome would auto apply the header for us): d354f56#diff-195bc33b49d5867b3f0b7e71303727bad811474107d1ec281a1d14f8d855465dL36
However, as the target endpoints are on a different domain and not in our whitelist here: https://github.com/RedHatInsights/insights-chrome/blob/master/src/utils/iqeEnablement.ts#L17 - the auth header is not correctly applied.
For now I've added the logic we had back (although we should not be pulling the token from the global). I will open a tech debt issue to track improving this impl. Because these URLs are sensitive I have opted not to update our whitelist.
Screenshots
NA
Checklist ☑️