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

Adding protocol handlers #972

Open
wants to merge 34 commits into
base: main
Choose a base branch
from
Open

Adding protocol handlers #972

wants to merge 34 commits into from

Conversation

diekus
Copy link
Member

@diekus diekus commented Apr 26, 2021

Closes #863 (#969)

This is an update that brings the protocol_handlers member addition up to date with the new structure of the specification (as stated in #969, it has been reworked since the previous effort was based on a very old version of the manifest spec).

cc @fabiorocha @aarongustafson

This change (choose at least one, delete ones that don't apply):

  • Adds new normative requirements
  • Adds new normative recommendations or optional items

Implementation commitment (delete if not making normative changes):

If change is normative, and it adds or changes a member:

  • [updated JSON

Person merging, please make sure that commits are squashed with one of the following as a commit message prefix:

  • chore:
  • editorial:
  • BREAKING CHANGE:
  • And use none if it's a normative change

Preview | Diff


Preview | Diff

diekus added 2 commits April 15, 2021 15:19
Adds protocol_handlers member and protocolHandlerItem.
Adds protocol_handlers member to manifest file.
@diekus diekus requested a review from marcoscaceres April 26, 2021 09:37
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
diekus added 2 commits April 26, 2021 12:40
-removes return statements that exited the loop while processing protocol_handlers.
- Adds for each to its infra definition.
@diekus diekus requested a review from kenchris April 26, 2021 11:51
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
Copy link
Member

@marcoscaceres marcoscaceres left a comment

Choose a reason for hiding this comment

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

This is a good start, but it needs a lot tighter integration with registerProtocolHandler on the security side - they should both use the same underlying model, and I'm not getting a sense this does. The URL replacement parts are also quite scary - all this should go through HTML's "normalize protocol handler parameters". The examples already show how this would be open to abuse (e.g., using random protocols shouldn't work - as that open up a significant attack avenue).

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
diekus and others added 3 commits May 7, 2021 12:23
Co-authored-by: Marcos Cáceres <[email protected]>
Co-authored-by: Marcos Cáceres <[email protected]>
Co-authored-by: Marcos Cáceres <[email protected]>
@aarongustafson
Copy link
Collaborator

This is a good start, but it needs a lot tighter integration with registerProtocolHandler on the security side - they should both use the same underlying model, and I'm not getting a sense this does. The URL replacement parts are also quite scary - all this should go through HTML's "normalize protocol handler parameters".

@marcoscaceres In line 1070, @diekus’ proposal punts to the HTML spec for the process of registering a protocol handler (see "register a protocol handler"), which is where all of the normalization and security stuff takes place. Do you think that needs to be framed differently to make that connection more clear for implementors?

        <p>
          To process the the <var>processedProtocolHandlers</var> the user
          agent SHOULD [=register a protocol handler=] per item defined in the
          [=sequence=].
        </p>

The examples already show how this would be open to abuse (e.g., using random protocols shouldn't work - as that open up a significant attack avenue).

Perhaps I’m misreading your concern here, but the second bullet in Example 6 specifically calls out that attempted abuse (like attempting to associate with "store") would be thrown out. Am I missing something?

            <li>The second protocol handler would be ignored, as the protocol
            provided does not start with "web+" and is not part of the
            safelist.
            </li>

diekus added 2 commits May 17, 2021 18:46
updates spec to remove traces of the old WebIDL ways.
@diekus diekus requested review from kenchris and marcoscaceres May 17, 2021 19:46
index.html Outdated Show resolved Hide resolved
index.html Outdated
Comment on lines 1940 to 1942
The user agent MUST ask for permission when using a protocol
handler for the first time. This feature requires user interaction
and a script cannot communicate with another application on its
Copy link
Member

Choose a reason for hiding this comment

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

This seems odd (not the asking for permission part - but this doesn't feel like it's wired up properly)...

Using HTML+JS as an example, can you show me what would this look like in practice? If it's to do with "permissions", then I suspect we need some kind of Permissions Policy or Permissions API thing here.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hola Marcos, so the way this would work is that these registrations happen upon installation of the Web App. There is a prompt, and it is similar to the one that appears when registration is invoked from the .registerProtocolHandler(). In this case, where the registration happens from the manifest, the prompt appears the first time the protocol handler is invoked for a Web app. (It was a requirement from security/privacy folks).

image

Copy link
Member

@marcoscaceres marcoscaceres May 25, 2021

Choose a reason for hiding this comment

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

In this case, where the registration happens from the manifest, the prompt appears the first time the protocol handler is invoked for a Web app.

Sorry, I'm still having a hard time following. Can you walk me through the set up end-to-end?

How do I "invoke a protocol handler"? Do I click on a link on some website (appears to be the case from the HTML spec)?

I'm also confused about this in the HTML spec:

navigator.registerProtocolHandler('web+soup', 'soup?url=%s')

If I can register protocol handlers to do the same thing in HTML using JS, why do we need them in the manifest again? Does this just duplicate the functionality? Or does this work differently or provide some additional functionality that I'm not getting?

Sorry for the dumb questions. I'm just trying to understand.

Copy link
Member Author

@diekus diekus May 25, 2021

Choose a reason for hiding this comment

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

Hola again Marcos! Not sure which dumb questions you are referring to, they all seem important to be clear for me!

The difference between navigator.registerProtocolHandler() and having a protocol_handlers member in the manifest file is is mostly where/when is the scheme registration and invocation happening. Let me explain a bit further:

  • navigator.registerProtocolHandler(): once it is called it will immediately try to register the scheme and show the prompt displayed above. This is handy if I want to register a scheme anytime from HTML/JS. Handy AF.
  • protocol_handlers member in the manifest file: this will register a/several scheme(s) when a Web application is being installed. (register the installed Web app as a possible handler for that scheme). Here a noteworthy difference is that the prompt itself would appear once a user in a website on the browser invokes one of the registered schemes. For example, I installed the BatmanPWA and when I am browsing online I click on a 'web+batman://' link and then the prompt appears in the browser asking if the user wants to use that (registered) PWA. This happens the first time a user invokes a specific scheme. Very convenient indeed.

So in summary, it is a matter of where the registration happens, as it is more convenient for a user to decide whether they want to use an installed Web app when they run into a link that would require it instead of getting N prompts when trying to install a Web app (where N = number of valid items in the protocol_handlers member in the manifest).

Copy link
Collaborator

Choose a reason for hiding this comment

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

There is also the automatic unregistration that happens when a user uninstalls a webapp, which is nice :)

Copy link
Member

Choose a reason for hiding this comment

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

protocol_handlers member in the manifest file: this will register a/several scheme(s) when a Web application is being installed.

Ok, but isn't this in violation of the web's security/permission model? I.e., we only really allow things to be request permission at use time, not en mass at install time.

Consider, we've previously rejected similar proposals whereby one would do:

  features: ["geolocation", "camera", "something else"]

This feels very similar.

Here a noteworthy difference is that the prompt itself would appear once a user in a website on the browser invokes one of the registered schemes. For example, I installed the BatmanPWA and when I am browsing online I click on a 'web+batman://' link and then the prompt appears in the browser asking if the user wants to use that (registered) PWA. This happens the first time a user invokes a specific scheme. Very convenient indeed.

This is concerning though. If I install BatmanPWA, it could do:

   protocol_handlers: [
       /// ...1000 protocols here... MUAHAHAH! 
       /// Appear to handle everything, and show up everywhere!
    ]

@dmurph:

There is also the automatic unregistration that happens when a user uninstalls a webapp, which is nice :)

Agree, that's nice. But it does presume some tight integration at the OS level with the browser. IIRC (and this was long ago), Firefox couldn't support such things because it could only put a shortcut icon on the home screen, but it had no way of getting notified if a user had deleted the shortcut.

Copy link
Member Author

Choose a reason for hiding this comment

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

Hola Marcos,

protocol_handlers member in the manifest file: this will register a/several scheme(s) when a Web application is being installed.

Ok, but isn't this in violation of the web's security/permission model? I.e., we only really allow things to be request permission at use time, not en mass at install time.

It is as you are saying, the permission request happens when the user is using the protocol for the first time. We think the consent dialog on this first use is a good compromise between asking for permission for every protocol that an app wants to register and the security concerns of registering the protocols.

Consider, we've previously rejected similar proposals whereby one would do:

  features: ["geolocation", "camera", "something else"]

This feels very similar.

Here a noteworthy difference is that the prompt itself would appear once a user in a website on the browser invokes one of the registered schemes. For example, I installed the BatmanPWA and when I am browsing online I click on a 'web+batman://' link and then the prompt appears in the browser asking if the user wants to use that (registered) PWA. This happens the first time a user invokes a specific scheme. Very convenient indeed.

This is concerning though. If I install BatmanPWA, it could do:

   protocol_handlers: [
       /// ...1000 protocols here... MUAHAHAH! 
       /// Appear to handle everything, and show up everywhere!
    ]

The list of safe-listed protocols is rather small and they're very likely to have multiple handlers already defined among browser, native and installed web apps, which makes it unlikely that an app can take over everything. Would setting an arbitrary limit to the number of protocols that can be registered help?

@dmurph:

There is also the automatic unregistration that happens when a user uninstalls a webapp, which is nice :)

Agree, that's nice. But it does presume some tight integration at the OS level with the browser. IIRC (and this was long ago), Firefox couldn't support such things because it could only put a shortcut icon on the home screen, but it had no way of getting notified if a user had deleted the shortcut.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
diekus and others added 3 commits May 18, 2021 09:58
Co-authored-by: Marcos Cáceres <[email protected]>
Co-authored-by: Marcos Cáceres <[email protected]>
Co-authored-by: Marcos Cáceres <[email protected]>
@diekus
Copy link
Member Author

diekus commented Jun 7, 2021

@aarongustafson wrote:

@marcoscaceres In line 1070, @diekus’ proposal punts to the HTML spec for the process of registering a protocol handler (see "register a protocol handler"), which is where all of the normalization and security stuff takes place. Do you think that needs to be framed differently to make that connection more clear for implementors?

Yes. At at minimum, we need to deal with the errors thrown by "register a protocol handler" (that should be simple enough... "if it throws, continue".). Also, the "register a protocol handler" takes "normalizedScheme and normalizedURLString", which is normalization we don't do.

These above small things, but I want to make sure the integration is done as expected. Right now, it feels like we are not taking as much care as we should in passing the right things to HTML.

I have changed the processing to better align (uses the same normalization for scheme and url parameters as rph())

@diekus diekus requested a review from marcoscaceres June 7, 2021 21:40
@diekus
Copy link
Member Author

diekus commented Jun 14, 2021

friendly ping @marcoscaceres 👋🏼

@marcoscaceres
Copy link
Member

marcoscaceres commented Jun 15, 2021

Hi @diekus, thanks for continuing to work on this. I've update the initial comment to include the things that are missing still for us to proceed. In particular, we need backing from a second engine and no objections.

index.html Outdated Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
@marcoscaceres
Copy link
Member

Moving to general discussion here...

Would setting an arbitrary limit to the number of protocols that can be registered help?

It feels like it would still be potentially overwhelming.

This is why registration via the existing APIs seems favorable to me. It already supports arbitrary same-origin URL registrations with good error handling?

So, fundamental question is: what significant value does this add over the existing protocol handler API? Also, if the PWA is not installed, won't developers need to call .registerProtocolHandler() anyway to support users who don't want the app on their home screen, but still want protocol handler capabilities?

domenic pushed a commit to whatwg/html that referenced this pull request Jun 25, 2021
@diekus
Copy link
Member Author

diekus commented Jun 28, 2021

Hola @marcoscaceres.

So, fundamental question is: what significant value does this add over the existing protocol handler API?

We think it makes a lot of sense to allow a developer to register protocol handlers in the manifest. It is where they will also register file handlers, link handlers and share targets, all in a way that ties them up nicely with the application itself.

Additionally, it is appropriate because they are linked to the app lifecycle, providing deregistration of those schemes once the app is uninstalled (unlike registerProtocolHandler()).

From the UX side, if an application needed to register to handle N protocols upon installing, it's bad UX to prompt the user N times for the permissions. With the protocols installed from the manifest, the user would only get a permission prompt the first time they link on a link.

Finally, we think it's about the constant evolution in HTML to make things more declarative. With the protocol_handlers member in the manifest, you don't need to rely on JS to register schemes for your installed app.

Also, if the PWA is not installed, won't developers need to call .registerProtocolHandler() anyway to support users who don't want the app on their home screen, but still want protocol handler capabilities?

Yes, but both are valid. I may want to have both the "mailclient.com" in the browser and additionally have the "Mail Client PWA" registered as a handler for mailto://. (one through registerProtocolHandler() and the other one through the manifest).

@diekus diekus requested a review from marcoscaceres June 28, 2021 10:54
@marcoscaceres
Copy link
Member

We think it makes a lot of sense to allow a developer to register protocol handlers in the manifest. It is where they will also register file handlers, link handlers and share targets, all in a way that ties them up nicely with the application itself.

This makes sense, particularly for new types (e.g., share target). There's been hesitancy amongst implementers in the past to duplicate functionality already provided by a JS API. Having said that...

Additionally, it is appropriate because they are linked to the app lifecycle, providing deregistration of those schemes once the app is uninstalled (unlike registerProtocolHandler()).

That's compelling, indeed - although the site could call unregisterProtocolHandler(). I guess this is interesting if protocols are "provisionally registered", but are inert until activated.

As an aside, we probably want to be clear with what happens when .unregisterProtocolHandler() is called, if it's not already.

From the UX side, if an application needed to register to handle N protocols upon installing, it's bad UX to prompt the user N times for the permissions. With the protocols installed from the manifest, the user would only get a permission prompt the first time they link on a link.

And this would only be for that particular link type, right? Not a blanket approval for all registered things?

Finally, we think it's about the constant evolution in HTML to make things more declarative. With the protocol_handlers member in the manifest, you don't need to rely on JS to register schemes for your installed app.

There are a lot of pros/cons here (personally, I like the imperative APIs over the declarative ones) - but let's keep this outside.

@marcoscaceres
Copy link
Member

marcoscaceres commented Jun 30, 2021

@diekus, could you please run HTM5 tidy on PR? There seem to be a couple of tiny markup issues:

line 1035 column 13 - Warning: missing </ol> before </li>
line 1052 column 11 - Warning: discarding unexpected </li>
line 1011 column 7 - Warning: missing </section> before <li>
line 1053 column 11 - Warning: inserting implicit <ul>
line 1056 column 7 - Warning: discarding unexpected </ol>
line 1053 column 11 - Warning: missing </ul> before </section>

diekus added 2 commits July 5, 2021 15:13
run tidy on manifest draft
runs tidy with the correct tidy config file 👀👀
@diekus
Copy link
Member Author

diekus commented Jul 5, 2021

We think it makes a lot of sense to allow a developer to register protocol handlers in the manifest. It is where they will also register file handlers, link handlers and share targets, all in a way that ties them up nicely with the application itself.

This makes sense, particularly for new types (e.g., share target). There's been hesitancy amongst implementers in the past to duplicate functionality already provided by a JS API. Having said that...

Additionally, it is appropriate because they are linked to the app lifecycle, providing deregistration of those schemes once the app is uninstalled (unlike registerProtocolHandler()).

That's compelling, indeed - although the site could call unregisterProtocolHandler(). I guess this is interesting if protocols are "provisionally registered", but are inert until activated.

As an aside, we probably want to be clear with what happens when .unregisterProtocolHandler() is called, if it's not already.

So registering protocol handlers from the manifest file would register them for the installed Web app. Registering protocol handlers through registerProtocolHandler() would register the website through the browser, therefore calling unregisterProtocolHandler() would not unregister any handlers from installed Web apps. In a way, the two ways of registering handlers are different because they act on different scopes.

From the UX side, if an application needed to register to handle N protocols upon installing, it's bad UX to prompt the user N times for the permissions. With the protocols installed from the manifest, the user would only get a permission prompt the first time they link on a link.

And this would only be for that particular link type, right? Not a blanket approval for all registered things?

Correct, it would be only for the particular link type that is triggered. Security wise, every different protocol registration needs to be granted permission on first use of that particular link type.

Finally, we think it's about the constant evolution in HTML to make things more declarative. With the protocol_handlers member in the manifest, you don't need to rely on JS to register schemes for your installed app.

There are a lot of pros/cons here (personally, I like the imperative APIs over the declarative ones) - but let's keep this outside.

@lauramorinigo
Copy link

+1 to this initiative, it can be a good start point for other similar cases like RSS.

Copy link
Collaborator

@mgiuca mgiuca left a comment

Choose a reason for hiding this comment

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

Some drive-by comments.

<ol>
<li>Let <var>processedProtocolHandlers</var> be a new [=list=].
</li>
<li>[=list/For each=] <var>protocol_handler</var> (<a>protocol
Copy link
Collaborator

Choose a reason for hiding this comment

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

Be consistent with variable naming. Looks like we're using camelCase, so protocolHandler not protocol_handler.

Question for @kenchris : did this change recently? I'm looking at the shortcuts algorithm above which uses variables like processedShortcuts. I swear we used to use variable names with spaces in them, not camelCaseVariableNames.

index.html Show resolved Hide resolved
index.html Show resolved Hide resolved
index.html Outdated Show resolved Hide resolved
index.html Outdated
</li>
</ol>
<ul>
<li>[=list/For each=] |processedProtocolHandlers|, [=register a
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't think this can appear here without context (not part of any algorithm). It doesn't say when it should happen, or what the requirement level is (I think it should be a SHOULD; without that it implies MUST).

At a minimum, this should be in an algorithm "To register the protocol handlers for a processed manifest manifest", then you can properly index into that manifest in the for loop, e.g. "For each protocolHandler of manifest["protocol_handlers"]".

Ideally, this would be called from an algorithm that's invoked at install time, but I don't see one of those. So maybe just add a note to the "Installable web applications" section to say the user agent MAY [=register the protocol handlers=]".

Copy link
Member Author

Choose a reason for hiding this comment

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

I have added the SHOULD requirement level for the registration and fixed the loose ul element that made it appear as orphan. I have also added a note to the installable web applications section indicating that a protocol handler may be installed.

index.html Outdated
</section>
<section>
<h3>
Privacy and Security considerations
Copy link
Collaborator

Choose a reason for hiding this comment

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

I'm not sure this should go here. There's a main P&S section at the end of the document that collects all of these (this would be the only section to have it's own P&S subsection, other than Installable Web Apps, and that is probably a mistake too -- note that it is duplicated text from the end of the document).

However, I think P&S should only have non-normative discussion; a MUST requirement shouldn't be in it. So I would move this MUST requirement above to Handling a protocol launch. Then explain why it is so in the P&S section.

index.html Outdated
</p>
<p>
The user agent MUST ask for permission when using a protocol handler
for the first time. This feature requires user interaction and a
Copy link
Collaborator

Choose a reason for hiding this comment

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

I don't understand this second sentence. Are you trying to write a normative requirement that registration requires user interaction? Are you justifying the previous MUST requirement? Please rephrase this.

index.html Outdated Show resolved Hide resolved
diekus and others added 2 commits August 2, 2021 20:06
- fixes orphan ul element
- moves protocol handler privacy and security section to main P&S section
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.

7 participants