BUG Let any user call LoginSessionController

silverstripe / silverstripe-session-manager

Allow users to manage and revoke access to multiple login sessions across devices.

BSD 3-Clause "New" or "Revised" License

9 stars 7 forks source link

BUG Let any user call LoginSessionController #68

Closed maxime-rainville closed 3 years ago

maxime-rainville commented 3 years ago

Fixes #67 and fixes #45

This is a work in progress. Still need to add the following:

[x] Test deleting a session you don't have access to
[x] Test deleting a session that does not exist
[x] Test an anonymous user
[x] Update Behat test to use a none-admin account

clarkepaul commented 3 years ago

@maxime-rainville Do we test the use of "keep me signed in for 30 days"?

Is not selected, session only lasts xyz hours/days
Is selected, remains active for the full 30 days?

maxime-rainville commented 3 years ago

I've rebased the PR ... just need to tweak the behat test.

Do we test the use of "keep me signed in for 30 days"?

Is not selected, session only lasts xyz hours/days

Is selected, remains active for the full 30 days?

@clarkepaul This PR is specifically about handling what happens when you use the session manager file to invalidate a session. If you try to logout of a "Remember me session" that has experienced, two things might happen:

if the login session has been "garbage collected", you will get the "This session could not be found or is no longer active" message
- In most cases, you will never see those expired sessions unless you somehow manage to load the page in-between the session expiring and the garbage collector doing its job
if login session garbage collection has not been set up and you try to invalidate an expired session, it will delete the matching LoginSession object and you'll get a success message.

Is that what you wanted to know?

michalkleiner commented 3 years ago

What does MOG stand for @emteknetnz? Might be confusing for non-native speakers following/using Silverstripe when acronyms like that are used.

emteknetnz commented 3 years ago

Merge on green - it's a workflow thing

We use it when we want to approve code changes though travis has not yet finished running. If the build goes green then anyone with merge permissions can merge the PR, including the person who originally created the PR

maxime-rainville commented 3 years ago

While in theory, we would like to have the community be involved in every PR and issue, the unfortunate reality is that there's a number of us that work on the project full time and that we will inevitably develop our own lingo.

I don't think it's practical for us to manage every PR in way that any community member could readily jump on it and understand every little detail of the conversation. But it's a good reminder that a lot of our conversations are not always accessible or self-evident, and that we should look for opportunities to make things easier to understand.