Skip to content
Open
Changes from all commits
Commits
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
9 changes: 5 additions & 4 deletions vehicle/polestar/identity.go
Original file line number Diff line number Diff line change
Expand Up @@ -69,14 +69,15 @@ func (v *Identity) login() (*oauth2.Token, error) {
"client_id": {ClientID},
"redirect_uri": {RedirectURI},
"response_type": {"code"},
"scope": {"openid profile email"},
"state": {lo.RandomString(16, lo.AlphanumericCharset)},
"scope": {"openid", "profile", "email"},
"code_challenge": {oauth2.S256ChallengeFromVerifier(cv)},
"code_challenge_method": {"S256"},
}

// Request authorization URL with browser-like headers
uri := fmt.Sprintf("%s/as/authorization.oauth2?%s", OAuthURI, data.Encode())
Copy link
Member

Choose a reason for hiding this comment

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

Unrelated?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Correct, some changes I thought were relevant and forgot to undo. Changed back with 3ef936c


req, _ := request.New(http.MethodGet, uri, nil, map[string]string{
"Accept": "text/html,application/xhtml+xml,application/xml;",
})
Expand All @@ -93,7 +94,7 @@ func (v *Identity) login() (*oauth2.Token, error) {
return nil, err
}

matches := regexp.MustCompile(`(?:url|action):\s*"/as/(.+?)/resume/as/authorization\.ping"`).FindStringSubmatch(string(body))
matches := regexp.MustCompile(`(?:url|action):\s*"(.+)"`).FindStringSubmatch(string(body))
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): The updated regex is too broad and greedy, which may capture unintended URLs or paths.

The new pattern now matches any url or action and uses a greedy (.+), which, combined with later concatenation, can capture full absolute or unrelated URLs (e.g., logout). Please either make the group non-greedy (e.g. (.+?) or [^\"]+) and/or restrict it to the expected login/resume path (e.g., a relative path under /as/).


if len(matches) < 2 {
return nil, errors.New("could not find resume path")
Expand All @@ -106,7 +107,8 @@ func (v *Identity) login() (*oauth2.Token, error) {
"client_id": {ClientID},
}

uri = fmt.Sprintf("%s/as/%s/resume/as/authorization.ping", OAuthURI, matches[1])
uri = OAuthURI + "/" + strings.TrimLeft(matches[1], "/") // Remove leading /
Copy link
Contributor

Choose a reason for hiding this comment

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

issue (bug_risk): Blindly prefixing OAuthURI to the matched URL/path can break if the match is an absolute URL.

If matches[1] is a full URL (e.g. https://idp.example.com/as/...), concatenating OAuthURI + "/" + ... will yield something like https://oauth.example.com/https://idp.example.com/.... Consider parsing matches[1] (e.g. with url.Parse) and only prefixing OAuthURI when it’s a relative path (no scheme/host); use absolute URLs as-is.


req, _ = request.New(http.MethodPost, uri, strings.NewReader(data.Encode()), map[string]string{
"Content-Type": "application/x-www-form-urlencoded",
"Accept": "application/json",
Expand All @@ -118,7 +120,6 @@ func (v *Identity) login() (*oauth2.Token, error) {
}
defer resp.Body.Close()

// Extract authorization code from response
code := resp.Request.URL.Query().Get("code")
if code == "" {
return nil, errors.New("missing authorization code")
Expand Down
Loading