From 249a8a4724beb0a08ab8798c12245f445fc27c7c Mon Sep 17 00:00:00 2001 From: Jianpeng Chao Date: Tue, 25 Aug 2020 12:10:39 -0700 Subject: [PATCH] Fix if the first fetching public keys failed from Hydra Added retry when oidc verifier is not exists when new request comes PiperOrigin-RevId: 328377540 Change-Id: If3cc37ad7859347e1fa9c988b63e4cda9e79062d --- lib/auth/auth.go | 37 +++++++++++++++++---------- lib/verifier/oidc_jwt_sig_verifier.go | 2 +- 2 files changed, 25 insertions(+), 14 deletions(-) diff --git a/lib/auth/auth.go b/lib/auth/auth.go index 424e588e..06b8400b 100644 --- a/lib/auth/auth.go +++ b/lib/auth/auth.go @@ -115,7 +115,7 @@ type Checker struct { // level field. transformIdentity func(*ga4gh.Identity) *ga4gh.Identity // init the verifier.AccessTokenVerifier - init sync.Once + mutex sync.Mutex // access token verifier verifier verifier.AccessTokenVerifier // use userinfo instead of the token itself to verify access token. @@ -123,16 +123,27 @@ type Checker struct { } func (s *Checker) getVerifier(ctx context.Context) (verifier.AccessTokenVerifier, error) { - var err error - s.init.Do(func() { - s.verifier, err = verifier.NewAccessTokenVerifier(ctx, s.issuer, s.useUserinfoVerifyToken) - }) + // TODO: We use lazy load for verifier creation since now Hydra + // and IC/DAM are deployed in a same container. Hydra is not available before + // IC/DAM startup completed. We should separate Hydra and IC/DAM to 2 + // containers after that we should fetch oidc public key when service + // starts and exit with error. - if err != nil { - return nil, err + if s.verifier != nil { + return s.verifier, nil } - return s.verifier, nil + s.mutex.Lock() + defer s.mutex.Unlock() + + // s.verifier maybe available after the waiting. + if s.verifier != nil { + return s.verifier, nil + } + + var err error + s.verifier, err = verifier.NewAccessTokenVerifier(ctx, s.issuer, s.useUserinfoVerifyToken) + return s.verifier, err } // NewChecker creates checker for authorization check. @@ -144,11 +155,11 @@ func (s *Checker) getVerifier(ctx context.Context) (verifier.AccessTokenVerifier // transformIdentity: transform as needed, will run just after token convert to identity. func NewChecker(logger *logging.Client, issuer string, permissions *permissions.Permissions, fetchClientSecrets func() (map[string]string, error), transformIdentity func(*ga4gh.Identity) *ga4gh.Identity, useUserinfoVerifyToken bool) *Checker { return &Checker{ - logger: logger, - issuer: issuer, - permissions: permissions, - fetchClientSecrets: fetchClientSecrets, - transformIdentity: transformIdentity, + logger: logger, + issuer: issuer, + permissions: permissions, + fetchClientSecrets: fetchClientSecrets, + transformIdentity: transformIdentity, useUserinfoVerifyToken: useUserinfoVerifyToken, } } diff --git a/lib/verifier/oidc_jwt_sig_verifier.go b/lib/verifier/oidc_jwt_sig_verifier.go index a4b7dd73..2012d24b 100644 --- a/lib/verifier/oidc_jwt_sig_verifier.go +++ b/lib/verifier/oidc_jwt_sig_verifier.go @@ -35,7 +35,7 @@ type oidcJwtSigVerifier struct { func newOIDCSigVerifier(ctx context.Context, issuer string) (*oidcJwtSigVerifier, error) { p, err := oidc.NewProvider(ctx, issuer) if err != nil { - return nil, errutil.WithErrorReason(errCreateVerifierFailed, status.Errorf(codes.Unavailable, "create oidc failed: %v", err)) + return nil, errutil.WithErrorReason(errCreateVerifierFailed, status.Errorf(codes.Unavailable, "create oidc failed, usually caused by service does not able reach to Hydra jwks endpoint: %v", err)) } v := p.Verifier(&oidc.Config{