juju status as admin with non-default format fails on other users' models

[Paul Collins]

On our production Juju 2 controller, running 2.5.0, I noticed the strange behaviour noted below.

This seems to happen with all models that are not owned by the admin user.

I can't work out how to view a models' access, but since admin is a superuser I'm assuming this shouldn't matter.

juju-upgrader cannot deal with such models, and I'm sure other things must also be affected.

(prodstack-is-juju2controller)prodstack-is@wekufe:~/juju-upgrader$ juju whoami
Controller: prodstack-is
Model: controller
User: admin
(prodstack-is-juju2controller)prodstack-is@wekufe:~/juju-upgrader$ juju show-user admin
user-name: admin
display-name: admin
access: superuser
date-created: "2016-09-22"
last-connection: just now
(prodstack-is-juju2controller)prodstack-is@wekufe:~/juju-upgrader$ juju status -m stg-pes-certification/c3-testing
Model Controller Cloud/Region Version SLA Timestamp
c3-testing prodstack-is prodstack-45/bootstack-ps45 2.4.7 unsupported 02:05:08Z

Model "stg-pes-certification/c3-testing" is empty.
(prodstack-is-juju2controller)prodstack-is@wekufe:~/juju-upgrader$ juju status -m stg-pes-certification/c3-testing --format=yaml
ERROR permission denied (unauthorized access)
(prodstack-is-juju2controller)prodstack-is@wekufe:~/juju-upgrader$ juju status -m stg-pes-certification/c3-testing --format=json
ERROR permission denied (unauthorized access)
(prodstack-is-juju2controller)prodstack-is@wekufe:~/juju-upgrader$ _

[Tim Penhey (thumper)]

Is this changed behaviour? I don't recall if this was allowed before.

[Paul Collins (pjdc)]

I'm fairly sure this is new behaviour. juju-upgrader doesn't seem to have changed recently, and we've been using it to perform our controller/model upgrades for quite a while now. In any case it seems strange for a superuser to be able to request juju status in one format but not another.

[Paul Collins (pjdc)]

I just deployed a brand new 2.4.7 controller, added a user, created a model with said user as the owner, and all 3 variants worked. Following "juju upgrade-juju --agent-version=2.5.0" the non-tabular variants now fail.

[Paul Collins (pjdc)]

transcript: https://pastebin.canonical.com/p/KWS69KtcH7/

[Paul Collins (pjdc)]

We wondered if this is because the models are still on 2.4.7, but after upgrading my test model I still see the same behaviour:

stagingstack-is@wekufe:~$ juju upgrade-juju --agent-version 2.5.0 -m blorp/blorp
best version:
    2.5.0
started upgrade to 2.5.0
stagingstack-is@wekufe:~$ juju status -m blorp/blorp
Model Controller Cloud/Region Version SLA Timestamp
blorp ps4.5-bootstack-ps45 ps4.5/bootstack-ps45 2.5.0 unsupported 03:32:24Z

Model "blorp/blorp" is empty.
stagingstack-is@wekufe:~$ juju status -m blorp/blorp --format json
ERROR permission denied (unauthorized access)

[Joel Sing (jsing)]

FWIW it appears that this fails on a 2.5.0 controller against a 2.4.7 model, however once the model is upgraded to 2.5.0 it starts working again.

[Paul Collins (pjdc)]

The above is in fact not the case.

I also repeated the test with blorp being a non-empty model (I deployed cs:ubuntu) with the same results.

[John A Meinel (jameinel)]

my guess is that in 2.5 we started querying a different API than just
FullStatus (new presence code?) and maybe it isn't properly allowing
superusers access to the new API.

Its *possible* that
juju status --debug
or
juju status --debug --logging-config=TRACE

would point us quickly at what the new API is.

On Fri, Feb 8, 2019 at 8:20 AM Paul Collins <email address hidden>
wrote:

> The above is in fact not the case.
>
> I also repeated the test with blorp being a non-empty model (I deployed
> cs:ubuntu) with the same results.
>
> --
> You received this bug notification because you are subscribed to juju.
> Matching subscriptions: juju bugs
> https://bugs.launchpad.net/bugs/1815154
>
> Title:
> juju status as admin with non-default format fails on other users'
> models
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/juju/+bug/1815154/+subscriptions
>

[Joel Sing (jsing)]

It seems the relevant output from:

  juju status -m stg-pes-certification/c3-testing --format=json --debug --logging-config=TRACE

Is this piece:

...
06:02:09 TRACE juju.rpc.jsoncodec codec.go:225 -> {"request-id":2,"type":"Storage","version":4,"request":"ListFilesystems","params":{"filters":[{}]}}
06:02:09 TRACE juju.rpc.jsoncodec codec.go:120 <- {"request-id":2,"error":"permission denied","error-code":"unauthorized access","response":{}}
...

Full trace is here - https://pastebin.canonical.com/p/VXtvrCV56P/

[Tim Penhey (thumper)]

Ah... it does in fact.

Showing storage was added to status in 2.5. Clearly there is no facade
version check there.

Tim

On 13/02/19 6:09 PM, John A Meinel wrote:
> my guess is that in 2.5 we started querying a different API than just
> FullStatus (new presence code?) and maybe it isn't properly allowing
> superusers access to the new API.
>
> Its *possible* that
> juju status --debug
> or
> juju status --debug --logging-config=TRACE
>
> would point us quickly at what the new API is.
>
> On Fri, Feb 8, 2019 at 8:20 AM Paul Collins <email address hidden>
> wrote:
>
>> The above is in fact not the case.
>>
>> I also repeated the test with blorp being a non-empty model (I deployed
>> cs:ubuntu) with the same results.
>>
>> --
>> You received this bug notification because you are subscribed to juju.
>> Matching subscriptions: juju bugs
>> https://bugs.launchpad.net/bugs/1815154
>>
>> Title:
>> juju status as admin with non-default format fails on other users'
>> models
>>
>> To manage notifications about this bug go to:
>> https://bugs.launchpad.net/juju/+bug/1815154/+subscriptions
>>

[Ian Booth (wallyworld)]

Without digging too deep, it's not likely to be a facade version issue - status uses existing storage facade api calls that have been part of juju since 1.x and are already used with the juju list-storage command - status simply makes the same api calls as that other CLI and displays the same info.

It's more likely simply a permission check issue not encountered before - no one ever ran juju list-storage against a model they didn't own.

[Achilleas Anagnostopoulos (achilleasa)]

It looks like the admin user does not have read access to the model created by the other user. The permission checking code that fails is: https://github.com/juju/juju/blob/2.5/apiserver/facades/client/storage/storage.go#L121

Is it safe to assume that a controller admin has read (and maybe write as I can see a checkCanWrite() method below the one linked above) access to *all* models in the controller?

Also, storage listing is not available before 2.5 we should probably update the status cmd implementation to check `errors.IsNotSupported` when attempting to list storage as it will probably cause issues if you use a > 2.5.0 juju cli to query an older controller's status.

[Achilleas Anagnostopoulos (achilleasa)]

Just to add to the above comment: if you login as the user that created the model and
`juju grant admin read $model` then the admin user can `juju status -m $model --format yaml`

[Richard Harding (rharding)]

However, as a superuser I get access to all models. Now, we do things like
hide them from the superuser's default list of models so that they can have
their own set of things they work with day to day, but they can do things
like list-models --all and should be able to status a model by another user
as long as the refer to it by the full name juju status owner/modelname.

On Wed, Feb 13, 2019 at 12:30 PM Achilleas Anagnostopoulos <
<email address hidden>> wrote:

> Just to add to the above comment: if you login as the user that created
> the model and
> `juju grant admin read $model` then the admin user can `juju status -m
> $model --format yaml`
>
> --
> You received this bug notification because you are subscribed to juju.
> https://bugs.launchpad.net/bugs/1815154
>
> Title:
> juju status as admin with non-default format fails on other users'
> models
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/juju/+bug/1815154/+subscriptions
>

[Achilleas Anagnostopoulos (achilleasa)]

So, if we change the linked `canRead` permission checking code to something like "if can_read_model || has_superuser_access" would that be safe given that the superuser should be able to access all models anyway?

[Richard Harding (rharding)]

Yes, that's good. We just also need to make sure list-storage and other
storage specific commands work well as part of QA there.

On Wed, Feb 13, 2019 at 1:01 PM Achilleas Anagnostopoulos <
<email address hidden>> wrote:

> So, if we change the linked `canRead` permission checking code to
> something like "if can_read_model || has_superuser_access" would that be
> safe given that the superuser should be able to access all models
> anyway?
>
> ** Changed in: juju
> Assignee: (unassigned) => Achilleas Anagnostopoulos (achilleasa)
>
> --
> You received this bug notification because you are subscribed to juju.
> https://bugs.launchpad.net/bugs/1815154
>
> Title:
> juju status as admin with non-default format fails on other users'
> models
>
> To manage notifications about this bug go to:
> https://bugs.launchpad.net/juju/+bug/1815154/+subscriptions
>

[Achilleas Anagnostopoulos (achilleasa)]

Fixed with: https://github.com/juju/juju/pull/9756

[John A Meinel (jameinel)]

https://github.com/juju/juju/pull/9817 merges this into develop