Skip to content

Commit

Permalink
revert: reduce size of verifiers (#3875)
Browse files Browse the repository at this point in the history
feat: store client in challenge/verified and reduce DB load.
  • Loading branch information
aeneasr authored Nov 5, 2024
1 parent d5f65c5 commit 7b82361
Show file tree
Hide file tree
Showing 4 changed files with 1 addition and 37 deletions.
5 changes: 0 additions & 5 deletions consent/handler.go
Original file line number Diff line number Diff line change
Expand Up @@ -1053,10 +1053,5 @@ func (h *Handler) decodeFlowWithClient(ctx context.Context, challenge string, op
return nil, err
}

f.Client, err = h.r.ClientManager().GetConcreteClient(ctx, f.ClientID)
if err != nil {
return nil, err
}

return f, nil
}
11 changes: 0 additions & 11 deletions consent/strategy_default.go
Original file line number Diff line number Diff line change
Expand Up @@ -348,11 +348,6 @@ func (s *DefaultStrategy) verifyAuthentication(
return nil, errorsx.WithStack(fosite.ErrAccessDenied.WithHint("The login verifier is invalid."))
}

f.Client, err = s.r.ClientManager().GetConcreteClient(ctx, f.ClientID)
if err != nil {
return nil, err
}

session, err := s.r.ConsentManager().VerifyAndInvalidateLoginRequest(ctx, verifier)
if errors.Is(err, sqlcon.ErrNoRows) {
return nil, errorsx.WithStack(fosite.ErrAccessDenied.WithHint("The login verifier has already been used, has not been granted, or is invalid."))
Expand Down Expand Up @@ -657,12 +652,6 @@ func (s *DefaultStrategy) verifyConsent(ctx context.Context, _ http.ResponseWrit
if err != nil {
return nil, nil, errorsx.WithStack(fosite.ErrAccessDenied.WithHint("The consent verifier has already been used, has not been granted, or is invalid."))
}

f.Client, err = s.r.ClientManager().GetConcreteClient(ctx, f.ClientID)
if err != nil {
return nil, nil, err
}

if f.Client.GetID() != r.URL.Query().Get("client_id") {
return nil, nil, errorsx.WithStack(fosite.ErrInvalidClient.WithHint("The flow client id does not match the authorize request client id."))
}
Expand Down
6 changes: 1 addition & 5 deletions flow/flow.go
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,7 @@ type Flow struct {
// Client is the OAuth 2.0 Client that initiated the request.
//
// required: true
Client *client.Client `db:"-" json:"client,omitempty"`
Client *client.Client `db:"-" json:"c,omitempty"`
ClientID string `db:"client_id" json:"ci,omitempty"`

// RequestURL is the original OAuth 2.0 Authorization URL requested by the OAuth 2.0 client. It is the URL which
Expand Down Expand Up @@ -514,7 +514,6 @@ func (f Flow) ToLoginChallenge(ctx context.Context, cipherProvider CipherProvide
if f.Client != nil {
f.ClientID = f.Client.GetID()
}
f.Client = nil
return flowctx.Encode(ctx, cipherProvider.FlowCipher(), f, flowctx.AsLoginChallenge)
}

Expand All @@ -523,7 +522,6 @@ func (f Flow) ToLoginVerifier(ctx context.Context, cipherProvider CipherProvider
if f.Client != nil {
f.ClientID = f.Client.GetID()
}
f.Client = nil
return flowctx.Encode(ctx, cipherProvider.FlowCipher(), f, flowctx.AsLoginVerifier)
}

Expand All @@ -532,7 +530,6 @@ func (f Flow) ToConsentChallenge(ctx context.Context, cipherProvider CipherProvi
if f.Client != nil {
f.ClientID = f.Client.GetID()
}
f.Client = nil
return flowctx.Encode(ctx, cipherProvider.FlowCipher(), f, flowctx.AsConsentChallenge)
}

Expand All @@ -541,6 +538,5 @@ func (f Flow) ToConsentVerifier(ctx context.Context, cipherProvider CipherProvid
if f.Client != nil {
f.ClientID = f.Client.GetID()
}
f.Client = nil
return flowctx.Encode(ctx, cipherProvider.FlowCipher(), f, flowctx.AsConsentVerifier)
}
16 changes: 0 additions & 16 deletions persistence/sql/persister_consent.go
Original file line number Diff line number Diff line change
Expand Up @@ -198,10 +198,6 @@ func (p *Persister) GetFlowByConsentChallenge(ctx context.Context, challenge str
if f.RequestedAt.Add(p.config.ConsentRequestMaxAge(ctx)).Before(time.Now()) {
return nil, errorsx.WithStack(fosite.ErrRequestUnauthorized.WithHint("The consent request has expired, please try again."))
}
f.Client, err = p.GetConcreteClient(ctx, f.ClientID)
if err != nil {
return nil, err
}

return f, nil
}
Expand Down Expand Up @@ -266,10 +262,6 @@ func (p *Persister) GetLoginRequest(ctx context.Context, loginChallenge string)
if f.RequestedAt.Add(p.config.ConsentRequestMaxAge(ctx)).Before(time.Now()) {
return nil, errorsx.WithStack(fosite.ErrRequestUnauthorized.WithHint("The login request has expired, please try again."))
}
f.Client, err = p.GetConcreteClient(ctx, f.ClientID)
if err != nil {
return nil, err
}
lr := f.GetLoginRequest()
// Restore the short challenge ID, which was previously sent to the encoded flow,
// to make sure that the challenge ID in the returned flow matches the param.
Expand Down Expand Up @@ -309,10 +301,6 @@ func (p *Persister) VerifyAndInvalidateConsentRequest(ctx context.Context, verif
if f.NID != p.NetworkID(ctx) {
return nil, errorsx.WithStack(sqlcon.ErrNoRows)
}
f.Client, err = p.GetConcreteClient(ctx, f.ClientID)
if err != nil {
return nil, err
}

if err = f.InvalidateConsentRequest(); err != nil {
return nil, errorsx.WithStack(fosite.ErrInvalidRequest.WithDebug(err.Error()))
Expand Down Expand Up @@ -359,10 +347,6 @@ func (p *Persister) VerifyAndInvalidateLoginRequest(ctx context.Context, verifie
if f.NID != p.NetworkID(ctx) {
return nil, errorsx.WithStack(sqlcon.ErrNoRows)
}
f.Client, err = p.GetConcreteClient(ctx, f.ClientID)
if err != nil {
return nil, err
}

if err := f.InvalidateLoginRequest(); err != nil {
return nil, errorsx.WithStack(fosite.ErrInvalidRequest.WithDebug(err.Error()))
Expand Down

0 comments on commit 7b82361

Please sign in to comment.