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

Add a capability indicating mandatory authentication #73

Open
emersion opened this issue Jun 10, 2021 · 13 comments · May be fixed by ircv3/ircv3-specifications#492
Open

Add a capability indicating mandatory authentication #73

emersion opened this issue Jun 10, 2021 · 13 comments · May be fixed by ircv3/ircv3-specifications#492

Comments

@emersion
Copy link

emersion commented Jun 10, 2021

On some setups, user authentication is mandatory. It would be nice if servers could communicate this to clients, e.g. via a sasl-required capability.

The use-case is properly asking the user for credentials if necessary (e.g. no "optional" indication in the password field).

@nillkitty
Copy link

nillkitty commented Sep 27, 2021

The issue with using capability nogotiation is that most clients do not wait for the server to send anything before sending their CAP, optional AUTH[ENTICATE]/PASS, NICK, and USER messages, so without a larger change on the client side, I don't know that you can really "inform" the client of much prior to its attempt to register (with or without authentication).

The way almost every other protocol (HTTP, SMB, SIP, ...) accomplishes this is to simply respond to an attempt to connect anonymously with an "authorization failed"-type response (think HTTP 401). The client attempts to connect anonymously, gets that error, and then goes through the supported authentication mechanisms until successful.

In the IRC world the only standard numeric reply I've ever seen for this is the ERR_NOACCESS (913) from IRCX (which arguably wasn't even needed in IRCX because IRCX clients are supposed to test for IRCX capabilities (and thus their auth requirements) before registering); but IRCv3 already reallocates most of IRCX's SASL-related numerics to lower numbers. Perhaps respond with ERR_SASLFAIL even though the user didn't AUTEHNTICATE, as if the user failed an implicit "anonymous" authentication attempt.

@jesopo
Copy link

jesopo commented Sep 27, 2021

The issue with using capability nogotiation is that most clients do not wait for the server to send anything before sending their CAP, optional AUTH[ENTICATE]/PASS, NICK, and USER messages, [...]

wh... what? clients that blindly CAP REQ are rare and reworking a client to not do that isn't a big job

@nillkitty
Copy link

nillkitty commented Sep 27, 2021

For example, mIRC sends CAP LS, NICK ..., and USER ... as soon as the socket connection is open. If you wanted to tell it that it needs to AUTHENTICATE first, you'd have to change the client to wait for a response to CAP which may never come (if the server doesn't support CAP) in order to know if it should authenticate or not before attempting to register.

I've never used a client that waits for the response to CAP because not all servers respond to CAP.

@jesopo
Copy link

jesopo commented Sep 27, 2021

you don't need to tell it to wait for a response to CAP LS that might never come, the server hangs open the connection registration attempt until you send CAP END so you just have to know to react to a CAP LS response if you get one, and otherwise just behave as normal. mIRC is a poor example of correct capability negotiation but even for it this would be easy

@nillkitty
Copy link

Ok that makes sense -- I didn't realize it held waiting for CAP END. In that case I agree.

@slingamn
Copy link

Ergo sends FAIL * ACCOUNT_REQUIRED in this case (when a client attempts to connect without SASL, but SASL is required).

I think this might be a better approach than an informational CAP, because whether SASL is required may depend on contingent aspects of the user's specific connection (what IP they're coming from, the result of a DNSBL lookup, etc.) and it's preferable not to have to vary the CAP response (or delay it, in the case of an external lookup) based on this.

I'm not sure about the stated rationale of prompting the user up front for a password, because in terms of clients that currently exist, I don't know how we could get the following combination of circumstances:

  1. The client knows that the server requires SASL (based on a previous connection attempt where it observed the sasl-required capability in CAP LS output? based on an in-progress connection attempt?)
  2. The end user has an account, but the client isn't aware of it

The flow where the client attempts to connect, receives FAIL * ACCOUNT_REQUIRED, and prompts the user accordingly (possibly in conjunction with the value of account-registration) seems just as good.

@emersion
Copy link
Author

For gamja, I had in mind to connect to the IRC server on page load just to grab the available caps, then disconnect immediately. Once the cap list is known, the client can decide:

  • Whether or not to display a password prompt at all (sasl advertised)
  • Whether or not to require the user to fill the password prompt (sasl-required advertised)
  • Whether or not to display a "Create account" button (register=before-connect)

Having the failure codes isn't as nice because it doesn't allow clients to show/hide login screen UI elements depending on server caps.

sasl-required would mean that auth is required for all users. If auth is required for some but not all users, gamja would need to display an optional password field, so same as sasl without sasl-required.

@slingamn
Copy link

For gamja, I had in mind to connect to the IRC server on page load just to grab the available caps, then disconnect immediately.

In order for gamja to know which server to connect to, it must be preconfigured with a config.json or a query string parameter, right? In which case, whoever's doing the configuration could also say whether SASL is required.

@emersion
Copy link
Author

gamja will default connecting to /socket if there's no config.json.

What I described isn't specific to gamja, it may be useful to other IRC clients as well, e.g. a future Android client.

@slingamn
Copy link

In the first case, whoever controls /socket could add a config.json and specify whether SASL is required.

But more generally, it seems that the flow goes like this:

  1. The client is informed of the server hostname or IRC URL, but not of nickname or account information
  2. The client connects to the server and retrieves the value of sasl-required
  3. The client adjusts its UI, requiring nickname information to proceed, but only requiring account information if sasl-required was advertised

but this seems only a marginal improvement on taking the nickname information up front, attempting connection registration, and then falling back to a UI that requires account information on observing FAIL * ACCOUNT_REQUIRED. (Handling this case is also more general, because it covers cases where SASL is required only due to a DNSBL or similar.) Compare HTTP basic auth, where the browser optimistically attempts the request and then pops up a user/password dialog on receiving a 401.

@emersion
Copy link
Author

Displaying a password entry and marking it as optional is misleading to users when the server requires auth or doesn't support auth.

@emersion
Copy link
Author

To be clear, I don't think FAIL * ACCOUNT_REQUIRED is a bad idea, on the contrary. It's just a different proposal which doesn't allow clients to offer the same UX.

@slingamn
Copy link

I tentatively withdraw my objections to this proposal and suggest the use of an additional CAP value field in draft/account-registration for this purpose, named account-required.

slingamn added a commit to slingamn/ircv3-specifications that referenced this issue Mar 15, 2022
Add `account-required` cap value to `draft/account-registration` as per
discussion here:

ircv3/ircv3-ideas#73
emersion added a commit to emersion/ircv3-specifications that referenced this issue Mar 25, 2022
emersion added a commit to emersion/ircv3-specifications that referenced this issue Apr 6, 2022
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 a pull request may close this issue.

4 participants