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.

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

SHS-5811: Add logout button to sites #1641

Open
wants to merge 6 commits into
base: develop
Choose a base branch
from

Conversation

codechefmarc
Copy link
Collaborator

@codechefmarc codechefmarc commented Oct 1, 2024

READY FOR REVIEW

Summary

  • Adds a temporary composer.json change for adding a forked version of the stanford_samlauth module that adds logout button functionality. (This change needs to be reverted before merge!)
  • Adds classes to the logout button to match login button

Need Review By (Date)

2024-09-07

Urgency

low

Steps to Test

  1. Login to the site as an admin
  2. Visit the Block layout page Block page: /admin/structure/block
  3. Click "Place block" in the footer region
  4. Choose the "SAML SUNetID Logout Block"
  5. Do not display the title, then click "Save Block"

Screenshot 2024-10-15 at 4 06 34 PM

  1. Verify on the front-end, while logged in, that there is now a logout button

Screenshot 2024-10-15 at 4 05 15 PM

  1. Click o the logout button and verify that it correctly logs you out of the site

PR Checklist


@ahughes3 ahughes3 temporarily deployed to Tugboat October 1, 2024 22:50 Destroyed
@ahughes3 ahughes3 temporarily deployed to Tugboat October 1, 2024 23:25 Destroyed
@codechefmarc codechefmarc self-assigned this Oct 1, 2024
@ahughes3 ahughes3 temporarily deployed to Tugboat October 2, 2024 18:24 Destroyed
@codechefmarc codechefmarc marked this pull request as ready for review October 2, 2024 18:48
@codechefmarc
Copy link
Collaborator Author

codechefmarc commented Oct 2, 2024

@joegl - Adding you in here for review. Other PR for the forked repo is here: SU-SWS/stanford_samlauth#21

@ahughes3 ahughes3 changed the title SHS-5811: Add "logout" button to sites SHS-5811: Add logout button to sites Oct 3, 2024
Base automatically changed from 11.2.5-release to develop October 3, 2024 15:39
@joegl joegl changed the base branch from develop to 11.3.1-release October 3, 2024 16:36
@ahughes3
Copy link
Collaborator

@joegl I added a comment - SU-SWS/stanford_samlauth#21 (comment) on the stanford_samlauth PR.

There was a comment that we needed clarification on: Since this is a "Login Block", it think it would be better to use a new block plugin.

@codechefmarc wasn't sure if this meant he needed to create a new block for logout as the original plan was to do everything in that block plugin.

I didn't know if it was more of a naming convention issue or a best practice/functionality issue. If the functionality works could updating the naming of the block to SamlLoginLogutBlock cover the concerns?

@ahughes3
Copy link
Collaborator

Update from Mike - We can change the name of the file & class but, it's not an option to change the ID of the block plugin. This would cause cascading issues on every platform that uses the module and block.
Because the block id is login, it is best to create a logout block. you can almost copy the current block plugin and just reverse the access condition and display a logout button instead.

@ahughes3 ahughes3 temporarily deployed to Tugboat October 15, 2024 21:30 Destroyed
@ahughes3 ahughes3 temporarily deployed to Tugboat October 15, 2024 22:19 Destroyed
@ahughes3 ahughes3 temporarily deployed to Tugboat October 15, 2024 22:20 Destroyed
@ahughes3 ahughes3 temporarily deployed to Tugboat October 15, 2024 22:25 Destroyed
@codechefmarc
Copy link
Collaborator Author

@cienvaras - Updated this PR with the latest code to split the logout block into its own plugin and ready for review.

Base automatically changed from 11.3.1-release to develop October 16, 2024 15:43
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants