Skip to content
This repository has been archived by the owner on May 8, 2024. It is now read-only.

feat: remember-me #17

Merged
merged 4 commits into from
Mar 20, 2024
Merged

feat: remember-me #17

merged 4 commits into from
Mar 20, 2024

Conversation

clovertera
Copy link
Contributor

@clovertera clovertera commented Mar 20, 2024

  • introduced new param to GenerateNewAccessToken()
  • if checkbox unchecked, user gets 1 hour
  • if checked, user gets 14 days

I may continue trying today to implement a checker func that login form can use to check for current cookie, get the username from id in cookie as well as remember bool, and then autofill login form with username if remember is true - probably not viable just coming up with ideas for it

@clovertera clovertera linked an issue Mar 20, 2024 that may be closed by this pull request
@@ -62,8 +62,15 @@ func Login(c *fiber.Ctx) error {
})
}

var timeToExpire int64
if login.Remember {
timeToExpire = 1209600 // 14 days
Copy link
Contributor

Choose a reason for hiding this comment

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

we should use that config value for remember

Copy link
Contributor Author

Choose a reason for hiding this comment

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

we should use that config value for remember

so in line 67 timeToExpire = config.Conf.JWTExpireSeconds ?
sorry I think I'm a bit confused, could you explain in more detail a bit?

Copy link
Contributor

Choose a reason for hiding this comment

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

yes

@BoYanZh
Copy link
Contributor

BoYanZh commented Mar 20, 2024

I may continue trying today to implement a checker func that login form can use to check for current cookie, get the username from id in cookie as well as remember bool, and then autofill login form with username if remember is true - probably not viable just coming up with ideas for it

and I think we do not need that checker function for now, we may do that later, it can be implemented in pure frontend

@clovertera
Copy link
Contributor Author

and I think we do not need that checker function for now, we may do that later, it can be implemented in pure frontend

oh really? i thought frontend cannot access cookies in localstorage so it would have to be in backend

@BoYanZh BoYanZh changed the title feat: remember-me (backend) feat: remember-me Mar 20, 2024
@BoYanZh BoYanZh merged commit 726dcd7 into main Mar 20, 2024
3 checks passed
@BoYanZh BoYanZh deleted the remember-me branch March 20, 2024 20:19
@BoYanZh
Copy link
Contributor

BoYanZh commented Mar 20, 2024

cookies and localstorage are two things. These JWT cookies are HttpOnly so they can not be accessed by js directly. But we can store the username in localstorage after success login, at that time, we request /users/me to get the username and store it in localstorage.

@clovertera
Copy link
Contributor Author

cookies and localstorage are two things. These JWT cookies are HttpOnly so they can not be accessed by js directly. But we can store the username in localstorage after success login, at that time, we request /users/me to get the username and store it in localstorage.

oh ok! thank you for explaining

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Remember me option [Bonus 2 pts)
2 participants