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

Default token params should not include a capabilities member #576

Closed
SimonWoolf opened this issue Jan 11, 2017 · 5 comments
Closed

Default token params should not include a capabilities member #576

SimonWoolf opened this issue Jan 11, 2017 · 5 comments
Labels
bug Something isn't working. It's clear that this does need to be fixed.

Comments

@SimonWoolf
Copy link
Member

By default, if using token auth (e.g. because they've specified a clientId), the lib is requesting a token with capabilities of "{\"*\":[\"*\"]}", see https://github.com/ably/ably-ios/blob/master/Source/ARTTokenParams.m#L32

This is wrong: it fails if someone is using a key with capabilities restricted to a subset of channels. (The user gets a Key does not support requested capabilities error right after initializing the library, which is a pretty bad user experience).

When no capability is specified by the user in defaultTokenParams, the token request should be sent with no capability member.

(RSA6 may be misleading, I'll file a docs issue)

@SimonWoolf SimonWoolf added the bug Something isn't working. It's clear that this does need to be fixed. label Jan 11, 2017
@mattheworiordan
Copy link
Member

@SimonWoolf see https://github.com/ably/realtime/issues/780. I am not sure I agree that request is wrong TBH. This is a bit contentious.

@SimonWoolf
Copy link
Member Author

@ricardopereira FYI: me Matt and Paddy discussed this (and Matt's concerns) today. We are going to put a workaround in realtime for this, and will be discussing deeper changes, but as far as iOS is concerned, this bug is correct, it should not be sending a capability member by default.

@funkyboy
Copy link
Contributor

@SimonWoolf Is this still happening?

@SimonWoolf
Copy link
Member Author

SimonWoolf commented Jan 31, 2018

@funkyboy I don't have a mac so can't verify directly, but the code in question doesn't appear to have been changed since I submitted this issue, so I assume it is still happening.

It's not that big a problem since we rewrote capability intersection in realtime to be commutative. But ably-ios behaviour is still wrong per the spec -- https://docs.ably.io/client-lib-development-guide/features/#RSA6 -- so should be fixed.

@SimonWoolf
Copy link
Member Author

@ricardopereira is there any update on this? This bug is two years old now, and is still causing customers problems from time to time.

Additionally, ably-ios is not properly encoding the capability in the query string e.g. &capability={%22*%22:%5B%22*%22%5D}&.... The braces aren't being encoded, when they should be.

Additionally, ably-ios is generating a timestamp and including it the querystring: &timestamp=1549468527.841610&format=.... This is wrong in three ways:

  1. It's in seconds (per spec should be in milliseconds since the epoch),
  2. It includes a fractional part (should be an integer number of milliseconds), and
  3. It shouldn't be there at all: per the spec (RSA9d) the timestamp should be generated in createTokenRequest by the auth server, not by the requesting client. (For one thing, a requesting client is quite likely to have a time that's off by >2 minutes).

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working. It's clear that this does need to be fixed.
Development

No branches or pull requests

4 participants