Skip to content

Commit

Permalink
fixes #2282 TOTP flow for OIDC was inconsistent
Browse files Browse the repository at this point in the history
- redirect on HTML/JSON
- fixes missing issuer context
- adds a test for cert + totp
  • Loading branch information
andrewpmartinez committed Aug 1, 2024
1 parent beebc1b commit 67e2660
Show file tree
Hide file tree
Showing 4 changed files with 102 additions and 16 deletions.
21 changes: 6 additions & 15 deletions controller/oidc_auth/login.go
Original file line number Diff line number Diff line change
Expand Up @@ -180,12 +180,6 @@ func renderPage(w http.ResponseWriter, pageTemplate *template.Template, id strin
}

func (l *login) checkTotp(w http.ResponseWriter, r *http.Request) {
responseType, err := negotiateResponseContentType(r)

if err != nil {
renderJsonApiError(w, err)
}

bodyContentType, err := negotiateBodyContentType(r)

if err != nil {
Expand Down Expand Up @@ -233,11 +227,8 @@ func (l *login) checkTotp(w http.ResponseWriter, r *http.Request) {
renderTotp(w, id, errors.New("invalid TOTP code"))
}

if responseType == HtmlContentType {
http.Redirect(w, r, l.callback(r.Context(), id), http.StatusFound)
}

renderJson(w, http.StatusOK, &rest_model.Empty{})
callbackUrl := l.callback(r.Context(), id)
http.Redirect(w, r, callbackUrl, http.StatusFound)
}

func (l *login) authenticate(w http.ResponseWriter, r *http.Request) {
Expand Down Expand Up @@ -284,6 +275,10 @@ func (l *login) authenticate(w http.ResponseWriter, r *http.Request) {
return
}

authRequest.SdkInfo = credentials.SdkInfo
authRequest.EnvInfo = credentials.EnvInfo
authRequest.AuthTime = time.Now()

if authRequest.SecondaryTotpRequired && !authRequest.HasAmr(AuthMethodSecondaryTotp) {
w.Header().Set(TotpRequiredHeader, "true")
if responseType == HtmlContentType {
Expand All @@ -295,10 +290,6 @@ func (l *login) authenticate(w http.ResponseWriter, r *http.Request) {
return
}

authRequest.AuthTime = time.Now()
authRequest.SdkInfo = credentials.SdkInfo
authRequest.EnvInfo = credentials.EnvInfo

callbackUrl := l.callback(r.Context(), credentials.AuthRequestId)
http.Redirect(w, r, callbackUrl, http.StatusFound)
}
Expand Down
2 changes: 2 additions & 0 deletions controller/oidc_auth/provider.go
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,8 @@ func NewNativeOnlyOP(ctx context.Context, env model.Env, config Config) (http.Ha
return http.HandlerFunc(func(writer http.ResponseWriter, request *http.Request) {
r := request.WithContext(context.WithValue(request.Context(), contextKeyHttpRequest, request))
r = request.WithContext(context.WithValue(r.Context(), contextKeyTokenState, &TokenState{}))
r = request.WithContext(op.ContextWithIssuer(r.Context(), config.Issuer))

oidcHandler.ServeHTTP(writer, r)
}), nil

Expand Down
2 changes: 1 addition & 1 deletion tests/mfa_ziti_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -1077,7 +1077,7 @@ func Test_MFA(t *testing.T) {
}

func computeMFACode(secret string) string {
now := int64(time.Now().UTC().Unix() / 30)
now := time.Now().UTC().Unix() / 30
code := dgoogauth.ComputeCode(secret, now)

//pad leading 0s to 6 characters
Expand Down
93 changes: 93 additions & 0 deletions tests/sdk_auth_test.go
Original file line number Diff line number Diff line change
@@ -1,10 +1,13 @@
//go:build apitests

package tests

import (
"crypto/x509"
"github.com/golang-jwt/jwt/v5"
"github.com/google/uuid"
"github.com/openziti/edge-api/rest_client_api_client/current_api_session"
"github.com/openziti/edge-api/rest_client_api_client/current_identity"
"github.com/openziti/edge-api/rest_management_api_client/auth_policy"
"github.com/openziti/edge-api/rest_management_api_client/external_jwt_signer"
management_identity "github.com/openziti/edge-api/rest_management_api_client/identity"
Expand Down Expand Up @@ -62,6 +65,96 @@ func TestSdkAuth(t *testing.T) {
ctx.Req.NotNil(apiSession.GetToken())
})

t.Run("oidc client cert + TOTP MFA, ca pool on client can authenticate", func(t *testing.T) {
ctx.testContextChanged(t)

testIdMfa := ctx.AdminManagementSession.RequireNewIdentityWithOtt(true)
testIdMfaCerts := ctx.completeOttEnrollment(testIdMfa.Id)

certCreds := edge_apis.NewCertCredentials([]*x509.Certificate{testIdMfaCerts.cert}, testIdMfaCerts.key)

client := edge_apis.NewClientApiClient([]*url.URL{clientApiUrl}, ctx.ControllerConfig.Id.CA(), func(strings chan string) {
strings <- ""
})
apiSession, err := client.Authenticate(certCreds, nil)

ctx.Req.NoError(err)
ctx.Req.NotNil(client)
ctx.Req.NotNil(apiSession)
ctx.Req.NotNil(apiSession.GetToken())

t.Run("can enroll in MFA TOTP", func(t *testing.T) {
ctx.testContextChanged(t)

enrollMfaParams := &current_identity.EnrollMfaParams{}
enrollMfaResp, err := client.API.CurrentIdentity.EnrollMfa(enrollMfaParams, nil)

ctx.Req.NoError(err)
ctx.Req.NotNil(enrollMfaResp)
ctx.Req.NotNil(enrollMfaResp.Payload)
ctx.Req.NotNil(enrollMfaResp.Payload.Data)

detailMfaParams := &current_identity.DetailMfaParams{}
detailMfaResp, err := client.API.CurrentIdentity.DetailMfa(detailMfaParams, nil)
ctx.Req.NoError(err)
ctx.Req.NotNil(detailMfaResp)
ctx.Req.NotNil(detailMfaResp.Payload)
ctx.Req.NotNil(detailMfaResp.Payload.Data)
ctx.Req.NotEmpty(detailMfaResp.Payload.Data.ProvisioningURL)

parsedUrl, err := url.Parse(detailMfaResp.Payload.Data.ProvisioningURL)
ctx.Req.NoError(err)

queryParams, err := url.ParseQuery(parsedUrl.RawQuery)
ctx.Req.NoError(err)
secrets := queryParams["secret"]
ctx.Req.NotNil(secrets)
ctx.Req.NotEmpty(secrets)

mfaSecret := secrets[0]

code := computeMFACode(mfaSecret)

verifyMfaParams := &current_identity.VerifyMfaParams{
MfaValidation: &rest_model.MfaCode{
Code: ToPtr(code),
},
}

verifyMfaResp, err := client.API.CurrentIdentity.VerifyMfa(verifyMfaParams, nil)
ctx.Req.NoError(err)
ctx.Req.NotNil(verifyMfaResp)

t.Run("can authenticate with newly enrolled TOTP MFA", func(t *testing.T) {
ctx.testContextChanged(t)

client := edge_apis.NewClientApiClient([]*url.URL{clientApiUrl}, ctx.ControllerConfig.Id.CA(), func(strings chan string) {
parsedUrl, err := url.Parse(detailMfaResp.Payload.Data.ProvisioningURL)
ctx.Req.NoError(err)

queryParams, err := url.ParseQuery(parsedUrl.RawQuery)
ctx.Req.NoError(err)
secrets := queryParams["secret"]
ctx.Req.NotNil(secrets)
ctx.Req.NotEmpty(secrets)

mfaSecret := secrets[0]

code := computeMFACode(mfaSecret)

strings <- code
})
client.SetUseOidc(true)
apiSession, err := client.Authenticate(certCreds, nil)

ctx.Req.NoError(err)
ctx.Req.NotNil(client)
ctx.Req.NotNil(apiSession)
ctx.Req.NotNil(apiSession.GetToken())
})
})
})

t.Run("client updb, ca pool on cert can authenticate", func(t *testing.T) {
ctx.testContextChanged(t)

Expand Down

0 comments on commit 67e2660

Please sign in to comment.