Skip to content

fix: Update persister_oauth2.go to handle special character | coming in t… #3849

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 4 commits into
base: master
Choose a base branch
from
+40 −4
Open

fix: Update persister_oauth2.go to handle special character | coming in t… #3849

Changes from all commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
1606831
Update persister_oauth2.go to handle special character | coming in t…
Ajayn84 Sep 25, 2024
070518c
fix: Update persister_oauth2.go to handle special character | coming …
Ajayn84 Sep 25, 2024
64b6083
fix: update commit to fix review comments
Ajayn84 Sep 25, 2024
afe6d35
Merge branch 'master' into fix/escape_pipe_char_for_consent_flow
Ajayn84 Dec 2, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
44 changes: 40 additions & 4 deletions persistence/sql/persister_oauth2.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -118,8 +118,8 @@ func (p *Persister) sqlSchemaFromRequest(ctx context.Context, signature string,
RequestedAt: r.GetRequestedAt(),
InternalExpiresAt: sqlxx.NullTime(expiresAt),
Client: r.GetClient().GetID(),
Scopes: strings.Join(r.GetRequestedScopes(), "|"),
GrantedScope: strings.Join(r.GetGrantedScopes(), "|"),
Scopes: strings.Join(escapeDelimiter(r.GetRequestedScopes()), "|"),
Copy link
Author

@Ajayn84 Ajayn84 Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are escaping delimter "|" using queryescape and then joining with "|" delimiter, so that scopes with "|" are replaced with and then joined using "|"

GrantedScope: strings.Join(escapeDelimiter(r.GetGrantedScopes()), "|"),
GrantedAudience: strings.Join(r.GetGrantedAudience(), "|"),
RequestedAudience: strings.Join(r.GetRequestedAudience(), "|"),
Form: r.GetRequestForm().Encode(),
Expand Down Expand Up @@ -179,13 +179,23 @@ func (r *OAuth2RequestSQL) toRequest(ctx context.Context, session fosite.Session
return nil, errorsx.WithStack(err)
}

scopes, err := unescapeDelimiter(r.Scopes)
if err != nil {
return nil, errorsx.WithStack(err)
}

grantedScopes, err := unescapeDelimiter(r.GrantedScope)
if err != nil {
return nil, errorsx.WithStack(err)
}

return &fosite.Request{
ID: r.Request,
RequestedAt: r.RequestedAt,
// ExpiresAt does not need to be populated as we get the expiry time from the session.
Client: c,
RequestedScope: stringsx.Splitx(r.Scopes, "|"),
GrantedScope: stringsx.Splitx(r.GrantedScope, "|"),
RequestedScope: scopes,
Copy link
Author

@Ajayn84 Ajayn84 Jan 31, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we are splitting the data from db with "|" and then unescaping to get "|" back in the scopes if any

GrantedScope: grantedScopes,
RequestedAudience: stringsx.Splitx(r.RequestedAudience, "|"),
GrantedAudience: stringsx.Splitx(r.GrantedAudience, "|"),
Form: val,
Expand Down Expand Up @@ -612,3 +622,29 @@ func (p *Persister) DeleteAccessTokens(ctx context.Context, clientID string) (er
p.QueryWithNetwork(ctx).Where("client_id=?", clientID).Delete(&OAuth2RequestSQL{Table: sqlTableAccess}),
)
}

func escapeDelimiter(scopes []string) []string {
escapedScopes := make([]string, len(scopes))
for i, scope := range scopes {
if strings.Contains(scope, "|") {
escapedScopes[i] = url.QueryEscape(scope)
} else {
escapedScopes[i] = scope
}
}
return escapedScopes
}

func unescapeDelimiter(scopes string) ([]string, error) {
updatedScopes := stringsx.Splitx(scopes, "|")
if strings.Contains(scopes, "%26") {
for i, scope := range updatedScopes {
unescapedScope, err := url.QueryUnescape(scope)
if err != nil {
return nil, errors.Errorf("Error while url unescaping scope: %s", scope)
}
updatedScopes[i] = unescapedScope
}
}
return updatedScopes, nil
}
Loading