kosmos/akkounts
8
1
Fork 0
Code Issues 26 Pull Requests 1 Actions Packages Releases 12 Activity

Expire inactive sessions, optionally allow to stay signed in #82

Merged
raucao merged 3 commits from feature/8-session_timeouts into master 2023-03-31 07:58:25 +00:00
Conversation 2 Commits 3 Files Changed 10
+66 -29
raucao commented 2023-02-19 05:45:50 +00:00
Owner
Copy Link
Copy Source

When "remember me" is checked, keep user logged in for two weeks after last activity. Otherwise, expire the session after 30 minutes.

closes #8

When "remember me" is checked, keep user logged in for two weeks after last activity. Otherwise, expire the session after 30 minutes. closes #8
Screenshot from 2023-03-19 18-06-01.png
229 KiB
Screenshot from 2023-03-19 18-06-01.png
raucao added the kredits-1 label 2023-02-19 05:45:50 +00:00
bumi reviewed 2023-02-19 13:24:05 +00:00
app/javascript/controllers/notification_controller.js Outdated
@@ -6,1 +6,4 @@
connect() {
// Devise timeoutable ends up adding a second flash message without content
// TODO investigate bug
if (this.element.textContent.trim() == "true") return;
bumi commented 2023-02-19 13:17:30 +00:00
Owner
Copy Link
Copy Source

so the content has the value "true"?

so the content has the value "true"?
raucao commented 2023-02-19 14:02:36 +00:00
Author
Owner
Copy Link
Copy Source

Yeah, for that unexpected additional flash message. I figured it's OK to hotfix like this, since no message should ever just say "true".

Yeah, for that unexpected additional flash message. I figured it's OK to hotfix like this, since no message should ever just say "true".
config/initializers/devise.rb Outdated
@@ -230,3 +230,3 @@
# The time you want to timeout the user session without activity. After this
# time the user will be asked for credentials again. Default is 30 minutes.
# config.timeout_in = 30.minutes
config.timeout_in = 24.hours
bumi commented 2023-02-19 13:24:02 +00:00
Owner
Copy Link
Copy Source

I tend to have this number higher these days.

I tend to have this number higher these days.
👍 1
raucao commented 2023-02-19 14:04:34 +00:00
Author
Owner
Copy Link
Copy Source

What timeout would you propose?

(Keep in mind that this is the central account management app, where you can potentially take over a lot of services if you're able to access an active browser session.)

What timeout would you propose? (Keep in mind that this is the central account management app, where you can potentially take over a lot of services if you're able to access an active browser session.)
raucao commented 2023-03-19 06:41:32 +00:00
Author
Owner
Copy Link
Copy Source

@bumi Did you see my question here?

@bumi Did you see my question here?
bumi commented 2023-03-19 09:06:30 +00:00
Owner
Copy Link
Copy Source

yeah, not sure... it's always the balance between convenience that I don't have to login again and some "security" in case a user uses a "public" computer.

but yeah, leave it to a day. if the user does not visit the site again after a day they have to login again.

yeah, not sure... it's always the balance between convenience that I don't have to login again and some "security" in case a user uses a "public" computer. but yeah, leave it to a day. if the user does not visit the site again after a day they have to login again.
raucao commented 2023-03-19 11:14:01 +00:00
Author
Owner
Copy Link
Copy Source

Since I also wasn't quite happy with forcing the tradeoff this way, I have overhauled the sign-in form and added a remember-me button, using our new toggle switch component and Devise's Rememberable strategy. So now it's better both ways: expiring faster when needed, and staying logged in when the user thinks it's safe.

Updated PR description and added a screenshot.

@bumi I'll wait for last review and approval before merging.

Since I also wasn't quite happy with forcing the tradeoff this way, I have overhauled the sign-in form and added a remember-me button, using our new toggle switch component and Devise's Rememberable strategy. So now it's better both ways: expiring faster when needed, and staying logged in when the user thinks it's safe. Updated PR description and added a screenshot. @bumi I'll wait for last review and approval before merging.
raucao requested review from bumi 2023-02-23 23:41:07 +00:00
raucao force-pushed feature/8-session_timeouts from 344e4e5bcc to a8a8fba14c 2023-03-19 11:08:19 +00:00 Compare
raucao changed title from Add time limit for inactive sessions to Expire inactive sessions, optionally allow to stay signed in 2023-03-19 11:09:45 +00:00
raucao added kredits-2 and removed kredits-1 labels 2023-03-19 11:14:12 +00:00
raucao added the
release
minor
 label 2023-03-28 14:29:22 +00:00
raucao added the security label 2023-03-28 14:40:12 +00:00
raucao requested review from greg 2023-03-29 07:45:27 +00:00
raucao requested review from galfert 2023-03-29 07:45:27 +00:00
galfert approved these changes 2023-03-31 00:34:41 +00:00
raucao merged commit 324809f77e into master 2023-03-31 07:58:25 +00:00
raucao deleted branch feature/8-session_timeouts 2023-03-31 07:58:25 +00:00
Sign in to join this conversation.
Reviewers
No Reviewers
bumi
greg
galfert
Labels
Clear labels
integration
btcpay
integration
discourse
integration
ejabberd
integration
email
integration
ldap
integration
lndhub
integration
mastodon
integration
nostr
integration
remotestorage
bug
Something is not working
 design
Graphic/visual design
 dev environment
Config, builds, CI, deployment, etc.
 docs
Documentation
 duplicate
This issue or pull request already exists
 enhancement
Improving existing functionality
 feature
New functionality
 good first issue
Dive in, and start contributing
 idea
Something to consider
 invalid
Not a bug
 kredits-1
Small contribution
 kredits-2
Medium contribution
 kredits-3
Large contribution
 on hold
Currently not actionable
 ops
Manual IT ops activities
 question
Looking for an answer
release
major
release
minor
release
patch
security
All your base are belong to us
 ui/ux
User interface, process design, etc.
 wontfix
This won't be fixed
No labels kredits-2
release
minor
 security
Milestone
No items
No Milestone
Assignees
Clear assignees
bkero (bkero) bumi (bumi) fsmanuel (fsmanuel) galfert (galfert) greg (Greg) hueso (hueso) raucao (Râu Cao) slvrbckt (Nick Jennings)
No Assignees
3 Participants
Notifications
Due Date
No due date set.
Dependencies

No dependencies set.

Reference: kosmos/akkounts#82