Add support to login using identity from an identity-aware proxy #715

Closed
branchmispredictor wants to merge 9 commits from branchmispredictor/api:feature/identity-aware-proxy into main
2 changed files with 18 additions and 57 deletions
Showing only changes of commit 575414b329 - Show all commits

View File

@ -79,7 +79,6 @@ type AuthClaims struct {
// but an outside source is the final source-of-truth for auth (e.g. Identity-Aware Proxy auth)
type AuthProvider interface {
GetUser(echo.Context, *AuthClaims) (*user.User, error)

I think this should return a *user.User since the external auth is still represented as a user in Vikunja. It's not a "new type of authentication" like link share is.

I think this should return a `*user.User` since the external auth is still represented as a user in Vikunja. It's not a "new type of authentication" like link share is.
RenewToken(echo.Context, *AuthClaims) (string, error)
}
var authProviders = map[AuthType]AuthProvider{}
@ -230,11 +229,7 @@ func RenewToken(s *xorm.Session, c echo.Context) (token string, err error) {
return NewUserJWTAuthtoken(u)
}
if authProvider, ok := authProviders[claims.Type]; ok {
token, err := authProvider.RenewToken(c, claims)
if err != nil {
return "", err
}
return token, nil
return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "External auth types do not use JWT tokens."}
}
return "", echo.NewHTTPError(http.StatusBadRequest, models.Message{Message: "Invalid JWT token."})
}

View File

@ -48,33 +48,7 @@ func Init() {
auth.RegisterAuthMiddleware(auth.AuthTypeIAPUser, Middleware())
}
// NewIAPUserJWTAuthtoken generates and signes a new jwt token for a user
// These are intentionally short lived because they can be regenerated at
// any time from the IAP authn information. They are not related to
// session length and are only used to provide user info to the frontend
// and a hint to auth.go to retrieve auth data from the IAP.
func NewIAPUserJWTAuthtoken(u *user.User) (token string, err error) {
// Set claims
claims := newIAPUserJWTAuthClaims(u)
t := jwt.NewWithClaims(jwt.SigningMethodHS256, claims)
// Generate encoded token and send it as response.
return t.SignedString([]byte(config.ServiceJWTSecret.GetString()))
}
func newIAPUserJWTAuthClaims(u *user.User) (claims *auth.AuthClaims) {
return &auth.AuthClaims{
Type: auth.AuthTypeIAPUser,
UserID: u.ID,
UserUsername: u.Username,
UserEmail: u.Email,
UserName: u.Name,
StandardClaims: jwt.StandardClaims{
ExpiresAt: time.Now().Add(time.Minute * 5).Unix(),
},
}
}
// Get or generate a new user for the claims made by the IAP
func userForIAPClaims(cl *IAPClaims) (u *user.User, err error) {
s := db.NewSession()
defer s.Close()
@ -94,39 +68,31 @@ func userForIAPClaims(cl *IAPClaims) (u *user.User, err error) {
return u, nil
}
func (p IAPAuthProvider) RenewToken(c echo.Context, authClaims *auth.AuthClaims) (string, error) {
// Get user
u, err := p.GetUser(c, authClaims)
if err != nil {
return "", nil
// Create an AuthClaims object for a user dervived from an IAP identity
func newIAPUserJWTAuthClaims(u *user.User) (claims *auth.AuthClaims) {
return &auth.AuthClaims{
branchmispredictor marked this conversation as resolved Outdated

Please document this.

Please document this.
Type: auth.AuthTypeIAPUser,
UserID: u.ID,
UserUsername: u.Username,
branchmispredictor marked this conversation as resolved Outdated

Please make errors like this into custom error types which you'd then return.

Please make errors like this into custom error types which you'd then return.
UserEmail: u.Email,
UserName: u.Name,
}
// Create token
return NewIAPUserJWTAuthtoken(u)
}
// Get a web.Auth object from the identity that the IAP provides
func (p IAPAuthProvider) GetUser(c echo.Context, authClaims *auth.AuthClaims) (*user.User, error) {
s := db.NewSession()
defer s.Close()
// Get the user from the IAP identity
cl := c.Get(IAPClaimsContextKey).(*IAPClaims)
u, err := userForIAPClaims(cl)
if err != nil {
return nil, err
// The IAP middleware already checked and created a user if needed, no need to regenerate them

Is there a special reason this is a separate endpoint instead of reusing the existing token one? It seems like this will require a frontend change?

Is there a special reason this is a separate endpoint instead of reusing the existing token one? It seems like this will require a frontend change?

There's a problem with bootstrapping auth here. The way IAPs work is by setting an http header with claims to the downstream service (vikunja-api), however javascript or front-end code does not have access to http headers.

So on initial load, there is no cookie / vikunja-jwt set so the frontend does not see us as logged in. Similarly, the existing /user/token endpoint is also unavailable because it is behind the jwt middleware and requires a jwt cookie set. So this endpoint exists outside of the jwt middleware so it can be accessed from before a user is "logged in"

There's a problem with bootstrapping auth here. The way IAPs work is by setting an http header with claims to the downstream service (vikunja-api), however javascript or front-end code does not have access to http headers. So on initial load, there is no cookie / vikunja-jwt set so the frontend does not see us as logged in. Similarly, the existing `/user/token` endpoint is also unavailable because it is behind the jwt middleware and requires a jwt cookie set. So this endpoint exists outside of the jwt middleware so it can be accessed from before a user is "logged in"

So this is the endpoint to "translate" the http header from the IAP to a Vikunja token? Wouldn't it be possible to just extend the existing middleware to look for that http header? All that'd be left then would be to extend the frontend so it somehow checks if the user is logged in (maybe by calling /api/v1/user and see if there's a 200 response coming back and then setting the frontend state to "logged in"?)

So this is the endpoint to "translate" the http header from the IAP to a Vikunja token? Wouldn't it be possible to just extend the existing middleware to look for that http header? All that'd be left then would be to extend the frontend so it somehow checks if the user is logged in (maybe by calling `/api/v1/user` and see if there's a 200 response coming back and then setting the frontend state to "logged in"?)

What I can do, is abtract this away a bit and make this an /auth/externalprovider/loggedin endpoint, so any future external auth source might also reuse the same endpoint.

Was writing that before I saw the reply. Yes, this basically translates the http header from the IAP to a vikunja token. The only problem is that the jwt middleware will error out for /api/v1/user/* if the frontend does not provide an already valid jwt as a cookie

~~What I can do, is abtract this away a bit and make this an `/auth/externalprovider/loggedin` endpoint, so any future external auth source might also reuse the same endpoint.~~ Was writing that before I saw the reply. Yes, this basically translates the http header from the IAP to a vikunja token. The only problem is that the jwt middleware will error out for /api/v1/user/* if the frontend does not provide an already valid jwt as a cookie

How would that look in the frontend, would it call that endpoint every time on reload?

How would that look in the frontend, would it call that endpoint every time on reload?

Yes, or at least if not already logged in / if current jwt token is expired. Essentially it's similar to an open-id auth from the frontend's point of view except it 1) is hitting vikunja-backend directly instead of first hitting an external service and 2) it is automatically calling the endpoint instead of waiting for the openid button to be hit.

We can actually make num 2 optional, if for whatever reason we prefer being able to have both IAP login and local/openid logins too.

Yes, or at least if not already logged in / if current jwt token is expired. Essentially it's similar to an open-id auth from the frontend's point of view except it 1) is hitting vikunja-backend directly instead of first hitting an external service and 2) it is automatically calling the endpoint instead of waiting for the openid button to be hit. We can actually make num 2 optional, if for whatever reason we prefer being able to have both IAP login and local/openid logins too.

Then I'd prefer modifying the existing jwt middleware to accept a jwt token or an IAP header if no token is provided (and move all the logic to create a new user etc there as well). That way, we won't need the new endpoint and can modify the frontend only a bit to make a call to /user every time it is loaded, regardless if it has a jwt token in localstorage or not. I think that would make the IAP a lot more transparent to the frontend.

Then I'd prefer modifying the existing jwt middleware to accept a jwt token or an IAP header if no token is provided (and move all the logic to create a new user etc there as well). That way, we won't need the new endpoint and can modify the frontend only a bit to make a call to `/user` every time it is loaded, regardless if it has a jwt token in localstorage or not. I think that would make the IAP a lot more transparent to the frontend.

Okay, I'll give it a shot.

Okay, I'll give it a shot.

Following up just to make sure I capture the implicit decisions here, so you want for the backend:

  1. If a valid jwt token is presented from the frontend, use that (e.g. ignore the IAP header)
  2. If no valid jwt token exists, create one from the IAP header. In the backend, treat this newly generated jwt token the same as if it was provided by the frontend

In the frontend:

  1. Call /user/token on load -> will get the jwt token generated from the IAP header
Following up just to make sure I capture the implicit decisions here, so you want for the backend: 1. If a valid jwt token is presented from the frontend, use that (e.g. ignore the IAP header) 2. If no valid jwt token exists, create one from the IAP header. In the backend, treat this newly generated jwt token the same as if it was provided by the frontend In the frontend: 1. Call /user/token on load -> will get the jwt token generated from the IAP header

Almost, I wouldn't generate a jwt token from the backend at all but instead use the IAP header to authenticate the user.

That would look like this in the middleware:

  1. If a valid jwt token is presented from the frontend, use that
  2. If a valid IAP header is present, use that as a means to authenticate the user in place of a jwt token. This includes creating the user internally if none exists yet. There may be a few places which would need to be a bit more abstracted to not only rely on a jwt token to get the user information.

The frontend would then always call user to figure out if the user is authenticated or not. Currently it only does that if a jwt token is present in local storage.

Almost, I wouldn't generate a jwt token from the backend at all but instead use the IAP header to authenticate the user. That would look like this in the middleware: 1. If a valid jwt token is presented from the frontend, use that 2. If a valid IAP header is present, use that as a means to authenticate the user in place of a jwt token. This includes creating the user internally if none exists yet. There may be a few places which would need to be a bit more abstracted to not only rely on a jwt token to get the user information. The frontend would then always call [`user`](https://try.vikunja.io/api/v1/docs#tag/user/paths/~1user/get) to figure out if the user is authenticated or not. Currently it only does that if a jwt token is present in local storage.
// Just use the authClaims provided by the middleware
u = &user.User{

What if there's more than one Key? Is that even possible? In that case, should we still just use the first one or return a different error?

What if there's more than one Key? Is that even possible? In that case, should we still just use the first one or return a different error?
ID: authClaims.UserID,
Email: authClaims.UserEmail,
Username: authClaims.UserUsername,
Name: authClaims.UserName,
}
// Sanity check that the user the frontend thinks it has (the authClaims from the JWT it passed in)
// is the same as the user provided by the IAP.
if authClaims.UserID != u.ID {
return nil, ErrIAPUserFrontendMismatch{}
}
return u, nil
}
// Validates the claims in the jwt
// Validates the claims in the IAP jwt
// Matches the jwt-go Claims interface
func (c *IAPClaims) Valid() error {
// Validate that expiresAt and issuedAt are set and valid (with up to 1 minute of skew)