Skip to content

Commit

Permalink
Merge pull request #417 from MadAppGang/feature/audit
Browse files Browse the repository at this point in the history
Correct refresh token + audit
  • Loading branch information
hummerdmag authored Aug 17, 2024
2 parents 4164b0e + 8ccd92a commit cf26b21
Show file tree
Hide file tree
Showing 19 changed files with 213 additions and 93 deletions.
14 changes: 9 additions & 5 deletions jwt/service/jwt_token_service.go
Original file line number Diff line number Diff line change
Expand Up @@ -232,7 +232,7 @@ func (ts *JWTokenService) NewAccessToken(u model.User, scopes []string, app mode
lifespan = TokenLifespan
}

claims := model.Claims{
claims := &model.Claims{
Scopes: strings.Join(scopes, " "),
Payload: payload,
Type: tokenType,
Expand Down Expand Up @@ -282,7 +282,7 @@ func (ts *JWTokenService) NewRefreshToken(u model.User, scopes []string, app mod
lifespan = RefreshTokenLifespan
}

claims := model.Claims{
claims := &model.Claims{
Scopes: strings.Join(scopes, " "),
Payload: payload,
Type: model.TokenTypeRefresh,
Expand Down Expand Up @@ -343,9 +343,13 @@ func (ts *JWTokenService) RefreshAccessToken(refreshToken model.Token, tokenPayl
return nil, ErrInvalidUser
}

requestedScopes := strings.Split(claims.Scopes, " ")

scopes := model.AllowedScopes(requestedScopes, user.Scopes, app.Offline)

token, err := ts.NewAccessToken(
user,
strings.Split(claims.Scopes, " "),
scopes,
app,
false,
tokenPayload)
Expand Down Expand Up @@ -414,7 +418,7 @@ func (ts *JWTokenService) NewResetToken(userID string) (model.Token, error) {

lifespan := ts.resetTokenLifespan

claims := model.Claims{
claims := &model.Claims{
Type: model.TokenTypeReset,
StandardClaims: jwt.StandardClaims{
ExpiresAt: (now + lifespan),
Expand Down Expand Up @@ -446,7 +450,7 @@ func (ts *JWTokenService) NewWebCookieToken(u model.User) (model.Token, error) {
now := ijwt.TimeFunc().Unix()
lifespan := ts.resetTokenLifespan

claims := model.Claims{
claims := &model.Claims{
Type: model.TokenTypeWebCookie,
StandardClaims: jwt.StandardClaims{
ExpiresAt: (now + lifespan),
Expand Down
1 change: 1 addition & 0 deletions model/slice.go
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ package model

import "strings"

// SliceIntersect returns only items in as that are found in bs.
// simple intersection of two slices, with complexity: O(n^2)
// there is better algorithms around, this one is simple and scopes are usually 1-3 items in it
func SliceIntersect(a, b []string) []string {
Expand Down
15 changes: 14 additions & 1 deletion model/token.go
Original file line number Diff line number Diff line change
Expand Up @@ -38,7 +38,7 @@ type Token interface {
}

// NewTokenWithClaims generates new JWT token with claims and keyID.
func NewTokenWithClaims(method jwt.SigningMethod, kid string, claims jwt.Claims) *jwt.Token {
func NewTokenWithClaims(method jwt.SigningMethod, kid string, claims *Claims) *jwt.Token {
return &jwt.Token{
Header: map[string]interface{}{
"typ": "JWT",
Expand Down Expand Up @@ -181,3 +181,16 @@ type Claims struct {

// Full example of how to use JWT tokens:
// https://github.com/form3tech-oss/jwt-go/blob/master/cmd/jwt/app.go

func AllowedScopes(requestedScopes, userScopes []string, isOffline bool) []string {
scopes := []string{}
// if we requested any scope, let's provide all the scopes user has and requested
if len(requestedScopes) > 0 {
scopes = SliceIntersect(requestedScopes, userScopes)
}
if SliceContains(requestedScopes, "offline") && isOffline {
scopes = append(scopes, "offline")
}

return scopes
}
10 changes: 5 additions & 5 deletions storage/grpc/shared/server.go
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@ type GRPCServer struct {
func (m *GRPCServer) UserByPhone(ctx context.Context, in *proto.UserByPhoneRequest) (*proto.User, error) {
user, err := m.Impl.UserByPhone(in.Phone)
if err == model.ErrUserNotFound {
return toProto(user), status.Errorf(codes.NotFound, err.Error())
return toProto(user), status.Error(codes.NotFound, err.Error())
}
return toProto(user), err
}
Expand All @@ -31,31 +31,31 @@ func (m *GRPCServer) AddUserWithPassword(ctx context.Context, in *proto.AddUserW
func (m *GRPCServer) UserByID(ctx context.Context, in *proto.UserByIDRequest) (*proto.User, error) {
user, err := m.Impl.UserByID(in.Id)
if err == model.ErrUserNotFound {
return toProto(user), status.Errorf(codes.NotFound, err.Error())
return toProto(user), status.Error(codes.NotFound, err.Error())
}
return toProto(user), err
}

func (m *GRPCServer) UserByEmail(ctx context.Context, in *proto.UserByEmailRequest) (*proto.User, error) {
user, err := m.Impl.UserByEmail(in.Email)
if err == model.ErrUserNotFound {
return toProto(user), status.Errorf(codes.NotFound, err.Error())
return toProto(user), status.Error(codes.NotFound, err.Error())
}
return toProto(user), err
}

func (m *GRPCServer) UserByUsername(ctx context.Context, in *proto.UserByUsernameRequest) (*proto.User, error) {
user, err := m.Impl.UserByUsername(in.Username)
if err == model.ErrUserNotFound {
return toProto(user), status.Errorf(codes.NotFound, err.Error())
return toProto(user), status.Error(codes.NotFound, err.Error())
}
return toProto(user), err
}

func (m *GRPCServer) UserByFederatedID(ctx context.Context, in *proto.UserByFederatedIDRequest) (*proto.User, error) {
user, err := m.Impl.UserByFederatedID(in.Provider, in.Id)
if err == model.ErrUserNotFound {
return toProto(user), status.Errorf(codes.NotFound, err.Error())
return toProto(user), status.Error(codes.NotFound, err.Error())
}
return toProto(user), err
}
Expand Down
2 changes: 0 additions & 2 deletions test/docker-compose.yml
Original file line number Diff line number Diff line change
@@ -1,6 +1,4 @@
# this is compose for test environment

version: "3"
services:
minio:
image: quay.io/minio/minio:latest
Expand Down
4 changes: 2 additions & 2 deletions test/test.sh
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@ export IDENTIFO_STORAGE_MONGO_TEST_INTEGRATION=1
export IDENTIFO_STORAGE_MONGO_CONN="mongodb://admin:password@localhost:27017/billing-local?authSource=admin"
export IDENTIFO_REDIS_HOST="127.0.0.1:6379"

docker-compose up -d
docker compose up -d

sleep 1
echo "dependencies started"
Expand All @@ -20,6 +20,6 @@ go test -race -timeout=60s -count=1 ../...
test_exit=$?

# docker-compose down -v
docker-compose rm -s -f -v
docker compose rm -s -f -v

exit $test_exit
13 changes: 4 additions & 9 deletions web/api/2fa.go
Original file line number Diff line number Diff line change
Expand Up @@ -195,7 +195,7 @@ func (ar *Router) ResendTFA() http.HandlerFunc {

scopes := strings.Split(token.Scopes(), " ")

authResult, err := ar.loginFlow(app, user, scopes, nil)
authResult, _, err := ar.loginFlow(app, user, scopes, nil)
if err != nil {
ar.Error(w, locale, http.StatusInternalServerError, l.APIInternalServerErrorWithError, err)
return
Expand Down Expand Up @@ -265,14 +265,7 @@ func (ar *Router) FinalizeTFA() http.HandlerFunc {
return
}

scopes := []string{}
// if we requested any scope, let's provide all the scopes user has and requested
if len(d.Scopes) > 0 {
scopes = model.SliceIntersect(d.Scopes, user.Scopes)
}
if model.SliceContains(d.Scopes, "offline") && app.Offline {
scopes = append(scopes, "offline")
}
scopes := model.AllowedScopes(d.Scopes, app.Scopes, app.Offline)

tokenPayload, err := ar.getTokenPayloadForApp(app, user.ID)
if err != nil {
Expand Down Expand Up @@ -312,6 +305,8 @@ func (ar *Router) FinalizeTFA() http.HandlerFunc {
}
}

journal(user.ID, app.ID, "2fa_login", scopes)

ar.server.Storages().User.UpdateLoginMetadata(user.ID)
ar.ServeJSON(w, locale, http.StatusOK, result)
}
Expand Down
10 changes: 10 additions & 0 deletions web/api/audit.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,10 @@
package api

import "log"

func journal(userID, appID, action string, scopes []string) {
// TODO: Create an interface for the audit log
// Implement it for logging to stdout, a database, or a remote service
log.Printf("audit record | %s | userID=%s appID=%s scopes=%v\n",
action, userID, appID, scopes)
}
4 changes: 3 additions & 1 deletion web/api/federated_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -203,7 +203,7 @@ func (ar *Router) FederatedLoginComplete() http.HandlerFunc {
return
}

authResult, err := ar.loginFlow(app, user, fsess.Scopes, nil)
authResult, resultScopes, err := ar.loginFlow(app, user, fsess.Scopes, nil)
if err != nil {
ar.Error(w, locale, http.StatusInternalServerError, l.ErrorFederatedLoginError, err)
return
Expand All @@ -212,6 +212,8 @@ func (ar *Router) FederatedLoginComplete() http.HandlerFunc {
authResult.CallbackUrl = fsess.CallbackUrl
authResult.Scopes = fsess.Scopes

journal(user.ID, app.ID, "federated_login", resultScopes)

ar.ServeJSON(w, locale, http.StatusOK, authResult)
}
}
Expand Down
8 changes: 5 additions & 3 deletions web/api/federated_oidc_login.go
Original file line number Diff line number Diff line change
Expand Up @@ -237,7 +237,7 @@ func (ar *Router) OIDCLoginComplete(useSession bool) http.HandlerFunc {
// map OIDC scopes to Identifo scopes
requestedScopes = mapScopes(app.OIDCSettings.ScopeMapping, requestedScopes)

authResult, err := ar.loginFlow(app, user, requestedScopes, nil)
authResult, resultScopes, err := ar.loginFlow(app, user, requestedScopes, nil)
if err != nil {
ar.Error(w, locale, http.StatusInternalServerError, l.ErrorFederatedLoginError, err)
return
Expand All @@ -247,9 +247,11 @@ func (ar *Router) OIDCLoginComplete(useSession bool) http.HandlerFunc {
authResult.CallbackUrl = fsess.CallbackUrl
}

authResult.Scopes = requestedScopes
authResult.Scopes = resultScopes
authResult.ProviderData = *providerData

journal(user.ID, app.ID, "oidc_login", resultScopes)

ar.ServeJSON(w, locale, http.StatusOK, authResult)
}
}
Expand Down Expand Up @@ -366,7 +368,7 @@ func (ar *Router) completeOIDCAuth(

providerScope, ok := providerScopeVal.(string)
if !ok {
ar.logger.Printf("oidc returned scope is not string but %T %+v", providerScope, providerScope)
ar.logger.Printf("oidc returned scope is not string but %T %+v", providerScopeVal, providerScopeVal)
}

// Extract the ID Token from OAuth2 token.
Expand Down
19 changes: 15 additions & 4 deletions web/api/federated_oidc_login_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -19,9 +19,10 @@ import (
)

var testApp = model.AppData{
ID: "test_app",
Active: true,
Type: model.Web,
ID: "test_app",
Active: true,
Offline: true,
Type: model.Web,
OIDCSettings: model.OIDCSettings{
ProviderName: "test",
ClientID: "test",
Expand Down Expand Up @@ -62,6 +63,8 @@ func init() {
panic(err)
}

testServer.Storages().App.CreateApp(testApp)

rs := api.RouterSettings{
LoginWith: model.LoginWith{
FederatedOIDC: true,
Expand Down Expand Up @@ -228,12 +231,20 @@ func Test_Router_OIDCLogin_Complete_ByEmail(t *testing.T) {
}

func claimsFromResponse(t *testing.T, response []byte) jwt.MapClaims {
return claimsFromJSONResponse(t, "access_token", response)
}

func refreshClaimsFromResponse(t *testing.T, response []byte) jwt.MapClaims {
return claimsFromJSONResponse(t, "refresh_token", response)
}

func claimsFromJSONResponse(t *testing.T, token_field string, response []byte) jwt.MapClaims {
var token map[string]any

err := json.Unmarshal(response, &token)
require.NoError(t, err)

at := token["access_token"].(string)
at := token[token_field].(string)
require.NotEmpty(t, at)

c := jwt.MapClaims{}
Expand Down
29 changes: 19 additions & 10 deletions web/api/federated_oidc_server_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,6 @@ import (

jwt "github.com/golang-jwt/jwt/v4"
ijwt "github.com/madappgang/identifo/v2/jwt"
"github.com/madappgang/identifo/v2/model"
"github.com/madappgang/identifo/v2/web/api"
)

Expand Down Expand Up @@ -65,15 +64,25 @@ func testOIDCServer() (*httptest.Server, context.CancelFunc) {
})

mux.HandleFunc("/token", func(w http.ResponseWriter, r *http.Request) {
idt, err := model.NewTokenWithClaims(jwt.SigningMethodES256, "kid", jwt.MapClaims{
"sub": "abc",
"emails": []string{"some@example.com"},
"email": "some@example.com",
"iss": cfg.Issuer,
"aud": "test",
"exp": time.Now().Add(time.Hour).Unix(),
"iat": time.Now().Unix(),
}).SignedString(privateKey)
token := jwt.Token{
Header: map[string]interface{}{
"typ": "JWT",
"alg": jwt.SigningMethodES256.Alg(),
"kid": "kid",
},
Claims: jwt.MapClaims{
"sub": "abc",
"emails": []string{"some@example.com"},
"email": "some@example.com",
"iss": cfg.Issuer,
"aud": "test",
"exp": time.Now().Add(time.Hour).Unix(),
"iat": time.Now().Unix(),
},
Method: jwt.SigningMethodES256,
}

idt, err := token.SignedString(privateKey)
if err != nil {
panic(err)
}
Expand Down
10 changes: 6 additions & 4 deletions web/api/impersonate_as.go
Original file line number Diff line number Diff line change
Expand Up @@ -27,20 +27,20 @@ func (ar *Router) ImpersonateAs() http.HandlerFunc {
return
}

userID := tokenFromContext(r.Context()).UserID()
userID := tokenFromContext(ctx).UserID()
adminUser, err := ar.server.Storages().User.UserByID(userID)
if err != nil {
ar.Error(w, locale, http.StatusUnauthorized, l.ErrorStorageFindUserIDError, userID, err)
return
}

log.Println("admin for impersonation", adminUser.ID, adminUser.Scopes)

d := impersonateData{}
if ar.MustParseJSON(w, r, &d) != nil {
return
}

log.Println("admin for impersonation", adminUser.ID, adminUser.Scopes, "as", d.UserID)

user, err := ar.server.Storages().User.UserByID(d.UserID)
if err != nil {
ar.Error(w, locale, http.StatusUnauthorized, l.ErrorStorageFindUserIDError, d.UserID, err)
Expand All @@ -63,7 +63,7 @@ func (ar *Router) ImpersonateAs() http.HandlerFunc {
"impersonated_by": adminUser.ID,
}

authResult, err := ar.loginFlow(app, user, nil, ap)
authResult, resultScopes, err := ar.loginFlow(app, user, nil, ap)
if err != nil {
ar.Error(w, locale, http.StatusInternalServerError, l.ErrorAPILoginError, err)
return
Expand All @@ -72,6 +72,8 @@ func (ar *Router) ImpersonateAs() http.HandlerFunc {
// do not allow refresh for impersonated user
authResult.RefreshToken = ""

journal(userID, app.ID, "impersonated", resultScopes)

ar.ServeJSON(w, locale, http.StatusOK, authResult)
}
}
Expand Down
Loading

0 comments on commit cf26b21

Please sign in to comment.