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

Security improvements #68

Merged
merged 2 commits into from
Nov 26, 2024
Merged

Security improvements #68

merged 2 commits into from
Nov 26, 2024

Conversation

nepella
Copy link
Contributor

@nepella nepella commented Nov 26, 2024

(See commit messages.)

@wiltonsr
Copy link
Owner

Hi, @nepella

Thanks for your contribution.

Could you please run go mod tidy to add the new requirement github.com/gorilla/securecookie to the go.mod file and commit?

This will make the workflow pass.

This will cause cache cookies to be invalidated when Traefik is
restarted and won't work well where Traefik is clustered but is still
better than shipping with an insecure value.

Also, this is the HMAC key, not an encryption key. See the documentation
for 'securecookie.New'.
Previously, CacheTimeout only affected the expiration time sent to the
client; the code treated cookies as valid for 30 days.
@nepella
Copy link
Contributor Author

nepella commented Nov 26, 2024

My bad; I forgot to run go test myself. Let's try this one more time.

@wiltonsr wiltonsr merged commit 74349f3 into wiltonsr:main Nov 26, 2024
7 checks passed
@wiltonsr
Copy link
Owner

Available in v0.1.10 release.

@nepella
Copy link
Contributor Author

nepella commented Nov 26, 2024

Thanks much for getting this merged so quickly and for writing this plugin. I'd been using the Nginx-ldap-auth proof-of-concept with the forwardauth middleware for some low-risk internal sites. This is better written and easier to use, and, with bind mode, I don't even have to store a secret.

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

Successfully merging this pull request may close these issues.

2 participants