cassieq icon indicating copy to clipboard operation
cassieq copied to clipboard

Queue name permission granualrity should be by regex and not strict queue names

Open devshorts opened this issue 9 years ago • 12 comments

There is no way for someone to create permissions to access all queues in an account without using key based auth. Query param auth now requires you to create a query param auth signature with a particular queue.

We should instead make this a regex so we can support all queues, or only queues with a certain naming pattern, or just a specific queue.

This will be much more flexible. If someone doesn't provide a queue when generating auth we should allow access to all queues (i.e. default should be *)

Or, at the very least, if we dont have queue name in the signature param, then it allows access to all queues. Though I like the regex idea :)

devshorts avatar Feb 16 '16 01:02 devshorts

here is a test that can replicate.

    @Test
    public void test_query_auth_with_no_queue_info_does_not_restricts() throws IOException, InvalidKeyException, NoSuchAlgorithmException {
        final AdminClient adminClient = AdminClient.createClient(server.getAdminuri().toString());

        final AccountName accountName = AccountName.valueOf("test_query_auth_with_no_queue_info_does_not_restricts");
        final QueueName queueName = QueueName.valueOf("test_query_auth_with_no_queue_info_does_not_restricts");

        final Response<AccountDefinition> createAccount = adminClient.createAccount(accountName).execute();
        final AccountKey accountKey = createAccount.body().getKeys().get(WellKnownKeyNames.Primary.getKeyName());

        final CassieqApi fullAccessClient = CassieqApi.createClient(server.getBaseUri(), CassieqCredentials.key(accountName, accountKey));

        // make both queues exist
        fullAccessClient.createQueue(accountName, new QueueCreateOptions(queueName)).execute();

        final GetAuthQueryParamsRequest getAuthQueryParamsRequest =
                GetAuthQueryParamsRequest.builder()
                                         .accountName(accountName)
                                         .keyName(WellKnownKeyNames.Primary)
                                         .level(AuthorizationLevel.PutMessage)
                                         .build();

        final QueryAuthUrlResult result = adminClient.createPermissions(getAuthQueryParamsRequest).execute().body();

        final String queryParam = result.getQueryParam();

        final CassieqApi authedClient =
                CassieqApi.createClient(server.getBaseUri(),
                                        CassieqCredentials.signedQueryString(queryParam));

        final Response<ResponseBody> goodRequestIsAuthorized =
                authedClient.addMessage(accountName, queueName, "Hello queue").execute();

        assertThat(goodRequestIsAuthorized.isSuccess()).isTrue();
    }

devshorts avatar Feb 16 '16 01:02 devshorts

+1

tbconroy avatar Feb 16 '16 01:02 tbconroy

Partial fix in #108, fyi @tronroy, thanks for the report

jakeswenson avatar Feb 16 '16 03:02 jakeswenson

Instead of regex what do you think about glob patterns? Also i like @devshorts idea of accepting a list of globs

jakeswenson avatar Feb 16 '16 03:02 jakeswenson

I like glob a lot more than regex as well. Lets go with glob. If we go with glob, lets keep it simple and not have lists. Having a single glob will force people to make sane queue naming choices anyways :p

devshorts avatar Feb 16 '16 03:02 devshorts

Ok simple globs: * means 0 or more ? means 0 or 1

what should we glob between? . like RMQ? or . & _? Or should the above just be literal character globs?

How do we want to encourage queues to be named? cassieq.request.completed

I lean towards globing between . only (makes it simpler to reason)

Globs would expand like:

  • * would be regexified as .*
  • ? would be regexified as [^\.]*

a glob like cassieq.*.completed would match the above example

jakeswenson avatar Feb 16 '16 04:02 jakeswenson

Using ? for 0 or 1 sucks since this goes in the query string... ideas on anything else that means 0 or 1?

jakeswenson avatar Feb 16 '16 04:02 jakeswenson

maybe * for 0 or 1 terms and ** 0 or more terms terms = dot separated pieces.

jakeswenson avatar Feb 16 '16 04:02 jakeswenson

Hmm. I know + is kind of overloaded but what about + for zero or 1 and * for zero or more? Just not sure I like using multiples of a character for semantic meaning since it can be easily error prone

Or what about :

Since we are limiting characters we have a lot of room to play with any non alphanumerics or underscore/dash

On Feb 15, 2016, at 8:19 PM, Jake Swenson [email protected] wrote:

maybe * for 0 or 1 terms and ** 0 or more terms terms = dot separated pieces.

― Reply to this email directly or view it on GitHub.

devshorts avatar Feb 16 '16 04:02 devshorts

+ also doesn't play nice for urls (it transforms into space)

jakeswenson avatar Feb 16 '16 04:02 jakeswenson

i think * and ** might be best.

cassieq.**.completed

eh. i'm losing interest in this due to the complexity

jakeswenson avatar Feb 16 '16 04:02 jakeswenson

Let's table this for another day. the other option is list for queue names and no regex. If we decide on globs or other matching semantics later we can always do that. I agree, this is going to be complicated and requires some more discussion and thought

On Feb 15, 2016, at 8:31 PM, Jake Swenson [email protected] wrote:

i think * and ** might be best.

cassieq.**.completed

eh. i'm losing interest in this due to the complexity

― Reply to this email directly or view it on GitHub.

devshorts avatar Feb 16 '16 04:02 devshorts