Skip to content

AC-143 new api end point to impersonate user#85

Open
bharkarampudi wants to merge 4 commits into4.11-ippfrom
AC-143
Open

AC-143 new api end point to impersonate user#85
bharkarampudi wants to merge 4 commits into4.11-ippfrom
AC-143

Conversation

@bharkarampudi
Copy link

@bharkarampudi bharkarampudi commented Oct 12, 2023 •
edited
Loading

Description

This is a new Api endpoint that allows us to impersonate a user in the system. This endpoint should only be callable by an ‘admin’ and should return the same cookies that the Cloud Infrastructure UI returns,

Types of changes

  • Breaking change (fix or feature that would cause existing functionality to change)
  • New feature (non-breaking change which adds functionality)
  • Bug fix (non-breaking change which fixes an issue)
  • Enhancement (improves an existing feature and functionality)
  • Cleanup (Code refactoring and cleanup, that may add test cases)

Feature/Enhancement Scale or Bug Severity

Feature/Enhancement Scale

  • Major
  • Minor

Bug Severity

  • BLOCKER
  • Critical
  • Major
  • Minor
  • Trivial

Screenshots (if appropriate):

How Has This Been Tested?

@bharkarampudi bharkarampudi reopened this Oct 12, 2023
nlgordon
nlgordon reviewed Oct 24, 2023
View reviewed changes
Copy link

@nlgordon nlgordon left a comment

Choose a reason for hiding this comment

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

I'm struggling to see how this is limited to just root admins. What stops a normal user from accessing the new impersonateUser endpoint?

@bharkarampudi
Copy link
Author

bharkarampudi commented Oct 26, 2023

I'm struggling to see how this is limited to just root admins. What stops a normal user from accessing the new impersonateUser endpoint?

I need to still push the changes.

@bharkarampudi
Copy link
Author

bharkarampudi commented Oct 27, 2023 •
edited
Loading

I'm struggling to see how this is limited to just root admins. What stops a normal user from accessing the new impersonateUser endpoint?

I have pushed all the changes now.

nlgordon
nlgordon reviewed Nov 21, 2023
View reviewed changes
server/src/com/cloud/api/ApiServer.java
sessionkey = attrObj.toString();
}
}
response.setCookie(String.format("JSESSIONID=%s;account=%s;domainid=%s;role=%s;sessionkey=%s;timezone=%s;timezoneoffset=%s;userfullname=%s;userid=%s;username=%s",
Copy link

@nlgordon nlgordon Oct 31, 2023

Choose a reason for hiding this comment

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

This isn't actually enough to make the UI update all the various user fields. So when the the user goes to the UI they still have the name of the admin that was doing the impersonation.

Copy link
Author

@bharkarampudi bharkarampudi Dec 1, 2023 •
edited
Loading

Choose a reason for hiding this comment

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

Yeah, I agree. In order to reflect this change in the complete web UI page then there are some UI files need to be updated with the session cookie values. But I was asked to do only API session related changes. If UI changes are required I can do that. what is your recommendation on that?

@ggoodrich-ipp ggoodrich-ipp added final review PR is ready for final review and merge and removed final review PR is ready for final review and merge labels Nov 28, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@nlgordon nlgordon Awaiting requested review from nlgordon

At least 1 approving review is required to merge this pull request.

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@bharkarampudi @nlgordon @ggoodrich-ipp