Skip to content
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.

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

Polestar: Enhance consent handling during authentication process #17252

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

rostbeule
Copy link

Change Summary

Enhanced Error Handling

  • An additional check was added to extract the uid (user ID) through redirection in cases where the code (Authorization Code) is missing.
  • If the code is empty, it is likely that an additional user action to confirm (decline cookies) is required, and the uid is used to complete this step.

User Consent for Cookies

  • If the code is unavailable, a new POST request is sent with the extracted uid and a confirmation to decline cookies in order to proceed with the authentication process.
  • These changes improve handling for cases where the code is initially unavailable and allow for automation of the user interaction to confirm consent.

@membero
Copy link

membero commented Nov 14, 2024

Hi Rostbeule, in my test with your patch I get "code not found" from line 96. So your additional confirmConsentAndGetCode is never called.

Even if I comment line 95-97 I get "Failed to extract user ID: code not found". Seems like the param() looks for the code and not the uid.

// If the authorization code is empty, this indicates that user consent must be handled
// before the code can be obtained. The `confirmConsentAndGetCode` method is called as a
// workaround to guide the user through the consent process and retrieve the authorization code.
if code == "" {
Copy link

Choose a reason for hiding this comment

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

We will never reach this as in line 96 we will return due to the error of not having the code.

Copy link
Author

Choose a reason for hiding this comment

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

Damn it – I missed the early return!

Copy link

Choose a reason for hiding this comment

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

lgtm


// Step 1: Extract the user ID (UID) from the redirect parameters
var uid string
if uid, err := param(); err != nil || uid == "" {
Copy link

Choose a reason for hiding this comment

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

In my tests, there is no UID in the params, still trying to figure out why.

Copy link

Choose a reason for hiding this comment

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

We are never reading a "uid" so it also cannot be extracted. I guess there is still something missing to make that work

Copy link
Author

Choose a reason for hiding this comment

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

added

Copy link

Choose a reason for hiding this comment

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

Does that actually work for you @rostbeule ?

For me it never retrieves the uid. Unfortunately, I am not really good with Go and do not fully understand how the this InterceptRedirect works.

There is a rather simple Python script that works perfectly fine for me:
https://github.com/CONSULitAS/Polestar_2_MQTT_Docker/blob/450503db2d1d5fce857326214b5f7709c3dea43e/Polestar_2_MQTT.py#L87

Here you can see that the uid is extracted from the response header after making the post request to https://polestarid.eu.polestar.com/as/{path_token}/resume/as/authorization.ping

I am still trying to figure out the difference to this working example but again struggle with Go 😉

Copy link
Author

Choose a reason for hiding this comment

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

I can reassure you there. These are actually my first experiences with Go. Honestly, I tested it in C# and tried to transfer it to Go. But I was upfront about that 😅

Will have a look into that

@naltatis naltatis changed the title Enhance consent handling during authentication process Polestar: Enhance consent handling during authentication process Nov 14, 2024
@andig andig added the vehicles Specific vehicle support label Nov 14, 2024
@andig
Copy link
Member

andig commented Nov 14, 2024

Thank you for the PR! Afaiu, it does not seem to work yet?

@rostbeule
Copy link
Author

Thanks for your feedback. Since I'm quite busy at the moment, I'll probably only get to it on Sunday. I'll take another look then.


// Extract the user ID (UID) from the redirect parameters
var uid string
if uid, err := param(); err != nil || uid == "" {
Copy link
Member

Choose a reason for hiding this comment

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

This uid is NOT the one from the line above. Due to := you‘re „shadowing“ it. Use = instead!

Copy link
Author

Choose a reason for hiding this comment

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

Wow, I didn't expect that! Thanks for the tip. Learned something new :)
I'm curious to see what @loebse thinks about it.

Copy link

Choose a reason for hiding this comment

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

It is not enough to solve the problem of not reading the uid

The code proposal by @scyomantion works for me though. Are you making the changes accordingly @rostbeule?

@scyomantion
Copy link

following code works for me:
line 86 - 110:

uri = fmt.Sprintf("%s/as/%s/resume/as/authorization.ping?client_id=%s", OAuthURI, resume, OAuth2Config.ClientID)

	var code string
	var uid string
	v.Client.CheckRedirect = func(req *http.Request, via []*http.Request) error {
		code = req.URL.Query().Get("code")
		uid = req.URL.Query().Get("uid")
		return nil
	}
	defer func() { v.Client.CheckRedirect = nil }()

	if _, err = v.Post(uri, request.FormContent, strings.NewReader(params.Encode())); err != nil {
		return nil, err
	}

	// If the authorization code is empty, this indicates that user consent must be handled
	// before the code can be obtained. The `confirmConsentAndGetCode` method is called as a
	// workaround to guide the user through the consent process and retrieve the authorization code.
	if code == "" {
		code, err = v.confirmConsentAndGetCode(resume, uid)
		if err != nil {
			return nil, err
		}
	}

function confirmConsentAndGetCode:

func (v *Identity) confirmConsentAndGetCode(resume string, uid string) (string, error) {
	// Extract the user ID (UID) from the redirect parameters
	if uid == "" {
		return "", fmt.Errorf("Failed to extract user ID: %s", uid)
	}

	// Confirm user consent by submitting the consent form, which rejects cookies
	data := url.Values{
		"pf.submit": []string{"true"},
		"subject":   []string{uid},
	}

	// Retrieve the authorization code after consent has been confirmed
	var param request.InterceptResult
	v.Client.CheckRedirect, param = request.InterceptRedirect("code", true)
	defer func() { v.Client.CheckRedirect = nil }()

	// Make a POST request to confirm the user consent
	if _, err := v.Post(fmt.Sprintf("%s/as/%s/resume/as/authorization.ping", OAuthURI, resume), request.FormContent, strings.NewReader(data.Encode())); err != nil {
		return "", fmt.Errorf("Error confirming user consent to reject cookies during the authentication process: %w", err)
	}

	// Extract the authorization code from the response
	code, err := param()
	if err != nil || code == "" {
		return "", fmt.Errorf("Failed to extract authorization code: %w", err)
	}

	// Return the retrieved code
	return code, nil
}

@loebse
Copy link

loebse commented Nov 14, 2024

I can confirm this works for me as well @scyomantion !!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
vehicles Specific vehicle support
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants