canonical / identity-platform-admin-ui

Admin UI for the Canonical identity broker and identity provider solution
Other
6 stars 4 forks source link

feat: add encryption mechanism for auth cookies #329

Closed BarcoMasile closed 3 months ago

BarcoMasile commented 3 months ago

Description

This PR introduces both the interface EncryptInterface and its implementation. This is needed to allow symmetric encryption of string values. This PR also adopts this newly added object in the AuthCookieManager without changing its methods signatures, to get us one step closer to the implementation of the user session spec, which will happen during the rest of the activities in IAM-811.

The new secret key for cookie values encryption must be exactly 32 bytes, as specified in the OAUTH2.md doc.

Important note on symmetric encryption

The choice in this PR was to use the standard library, as the preferred strategy throughout the codebase. The standard library implementation of the AES algorithm with nonce allows for 16, 24 or 32 bytes for the secret key. The choice here is to go with 32 bytes.

BarcoMasile commented 3 months ago

suggestion: We could use https://github.com/gorilla/securecookie for signing and encrypting the cookies, to avoid having to deal with low level functions

So this seems nice, I have to admit. Thing is that switching to this right now would mean throw away 95% of the work done in this PR and starting over. I would probably consider this when/if we decide to support secret rotation for cookies encryption. The good thing is that the std library has not been too complicated to use

shipperizer commented 3 months ago

suggestion: We could use https://github.com/gorilla/securecookie for signing and encrypting the cookies, to avoid having to deal with low level functions

So this seems nice, I have to admit. Thing is that switching to this right now would mean throw away 95% of the work done in this PR and starting over. I would probably consider this when/if we decide to support secret rotation for cookies encryption. The good thing is that the std library has not been too complicated to use

let's add an issue on GitHub, if we can save ourselves from writing code we should (within reasons)

anyway lgtm

BarcoMasile commented 3 months ago

let's add an issue on GitHub, if we can save ourselves from writing code we should (within reasons)

anyway lgtm

@shipperizer Issue created: https://github.com/canonical/identity-platform-admin-ui/issues/330