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

Consent support chapter. #483

Merged
merged 6 commits into from
Nov 14, 2023
Merged

Consent support chapter. #483

merged 6 commits into from
Nov 14, 2023

Conversation

UlfBj
Copy link
Contributor

@UlfBj UlfBj commented Jul 14, 2023

No description provided.

Signed-off-by: Ulf Bjorkengren <[email protected]>
@UlfBj
Copy link
Contributor Author

UlfBj commented Jul 14, 2023

Signed-off-by: Ulf Bjorkengren <[email protected]>
@@ -1123,7 +1126,14 @@ <h2>Client</h2>
<li>The <a href="#device-roles-def">device</a>. It is in charge of running the Apps that make requests to the VISSv2 server</li>
<li>The <a href="#application-roles-def">app. </a>It runs requests on behalf of the user.</li>
<li>The <a href="#user-roles-def">user</a>. It delegates access rights to the app.</li>
</ul>
</ul>
Besides the three sub-acor roles the client is also characterized by a
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling error "sub-acor"

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed

are deployed in-vehicle the VIN parameter may be omitted, in all other deployment scenarios it shall be present.
are deployed in-vehicle the VIN parameter may be omitted, in all other deployment scenarios it shall be present.<br>
The purpose of the Client Id is to provide additional information to the consent end point that takes the decision on
granting consent or not. A too uninformative description may lead to consents being denied.
Copy link
Contributor

Choose a reason for hiding this comment

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

To me it is a bit unclear what is meant with "description". We do not intended to have a free text description as part of the Client Id, or?

Or are we missing a sentence like "The consent end point may based on the client id show a dialog to the vehicle owner"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

A link is added to the Client chapter, where more info about the Client Id can be found.

Regarding the possibly missing sentence, I think we should avoid saying anything about the ECF behavior.

However, a VISSv2 vehicle server has a capability to enforce consent results, i. e. to allow or block access to requested data.
This can be leveraged in a model where the server receives consent results from an “External Consent Framework” (ECF) and uses that information to either grant client requests,
or not, for data that is consent protected. How the ECF obtains the consent status is out-of-scope in this specification.
A secure, local communication channel exits between the in-vehicle ECF and the server as shown in the figure below,
Copy link
Contributor

Choose a reason for hiding this comment

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

Spelling error "exits"?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Fixed.

A secure, local communication channel exits between the in-vehicle ECF and the server as shown in the figure below,
over which the server can inquire about the consent status for data requested by a client.
<figure id="fig-consent-architecture">
<img src="images/consent-architecture.jpg" alt="Consent architecture.">
Copy link
Contributor

Choose a reason for hiding this comment

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

What is the source of the image? Would it be relevant to include the source format in the PR so that the image can be modified if needed?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The image should be found in the images directory. It was initially missing.

Copy link
Contributor

Choose a reason for hiding this comment

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

But the file images/consent-architecture.jpg is not the source format, or? Or have you used a tool that works directly on *.jpg files?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I created it in my favorite drawing tool, Paint :). So it is the source format.
My drawings belong to the naivism art direction, you are very welcome to redraw it in another art direction :).

@@ -1723,6 +1734,71 @@ <h2>Access Control Selection</h2>
</section>
</section>

<section id="consent-support">
<h2>Consent support</h2>
<p>
Copy link
Contributor

Choose a reason for hiding this comment

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

Do we need a "This section is non-normative." statement?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Could very well be so. Let us discuss that in the next WG meeting (cancelled next week).
As I believe it is more likely than not, I add it for now.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Signed-off-by: Ulf Bjorkengren <[email protected]>
</ul>
Besides the three sub-actor roles the client is also characterized by a
<ul>
<li>client identity.</li>
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we might need to talk about client registration and define the ClientID as a pointer to the data associated to it during registration. That way, tokens just need to include the clientid and the client context could be retrieved from the corresponding server using the clientid. It could be a local registration, but decoupling this process will simplify the description of the tokens and make it more clear, at least this is my opinion.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I agree with what you propose.
Would it be possible for you to provide an update with what you propose?

Copy link
Contributor

Choose a reason for hiding this comment

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

I can prepare another PR but before doing that it might be better to agree in the general changes. My proposal would be to state that "We assume all clients are registered in the ecosystem manager, providing information about the device and application, but also information need for consent management. Upon registration, all clients will receive a Client ID. All servers should be able get the client information from the Client ID. Client registration is out of the scope of this specification" If we do that, we could include in the tokens the client ID instead of the context. We will also need to change a bit the text. I think client registration should not involve the user, that leaves the question whether we need the user in the tokens or not. I think it is relevant for the access grant token and consent, but maybe not needed for the access token. This is something we may need to discuss in more detail ...

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@isaacagudo I think the statement makes things clear and concise, so I am backing adding that. Also then to replace the context with the client Id in the token.
I agree that the registration as described in the statement should not include the user, but that it is needed in the consent context. If it is needed in the consent, does that not imply that the access token would be an appropriate bearer of the information? The triggering of obtaining consent (if not already done and being valid) starts (as I see it?) with the server receiving a request containing an access token.

Copy link
Contributor

Choose a reason for hiding this comment

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

In my opinion consent should be triggered when requesting the access token, not when accessing the actual signals using the token, that why I suggested not including the user in the access token. However, it might be interesting to keep it for logging purposes.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

When thinking more about it, I agree that consent triggering is more appropriate when requesting the access token. But maybe we should keep the user in it anyway.

Copy link
Contributor

Choose a reason for hiding this comment

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

Yes, I agree we can keep the user anyway

<li>The consent status.</li>
<li>The expiry time of the consent status.</li>
</ul>
This communication shall be carried out on a confidentiality protected channel.
Copy link
Contributor

Choose a reason for hiding this comment

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

I think the chapter about consent is ok. I only have a comment about the security of the communication. It's better to just say "communication shall be carried out using a secure channel (e.g. TLS)". If we want to go into more details, we would also need authentication in the sense that the VISS server needs to be sure it's talking to the right ECF and viceversa.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I will update accordingly to your proposal.

Ulf Bjorkengren added 3 commits October 18, 2023 18:36
Signed-off-by: Ulf Bjorkengren <[email protected]>
Signed-off-by: Ulf Bjorkengren <[email protected]>
Signed-off-by: Ulf Bjorkengren <[email protected]>
@tguild tguild merged commit 65b7766 into w3c:gh-pages Nov 14, 2023
3 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants