On Tue, 8 Oct 2019 at 14:59, Marek Posolda <mposolda(a)redhat.com> wrote:
On 08. 10. 19 7:54, Stian Thorgersen wrote:
Ok, I wasn't aware that the old console was able to list applications the
user is not currently using. Testing it out I can see now it does indeed do
that, but that it is broken as it lists a lot of irrelevant clients.
What it should list is:
1. Applications currently in use - this will be any application registered
in the session (third party apps need an option to revoke access, which
will remove the granted consents)
2. Applications with offline access (these need to somehow be
differentiated from the above and have an option to revoke access, which
will remove the offline session)
3. Applications that are actual web applications and that are available to
the user
What we need to discuss is what to do in step 3. It is clear to me that
the logic in the old console is not working correctly, so we need a better
approach. What users need is the ability to discover applications they can
access from the account console, that means it should be web applications
with a baseUrl so there can be a link to open the application. It should
not list applications just because they require consent or just because
they can get an offline token, because that doesn't mean a user can
actually start using them. Further, it should be possible for a admin to
control what applications are listed there, which they can do based on what
applications users have access to and have a baseUrl set on them.
So assuming there are those two groups of clients:
(a) clients, which already has consent or offline access
(b) clients, which can get consent or offline access
Not quite. The groups are:
(a) clients that are already in use
(b) clients that have consent
(c) clients that have offline access
(d) clients that are web applications and the user can access (i.e. account
service can link to the app and the user can actually use the app)
clients should not be listed just because they can get consent or offline
access, those should only be listed if they fall in (d)
I think that we're in agreement that clients from group (a) with
already
available stuff should be displayed in new account console? As there should
be a way for the user to "Revoke" the consent or offline access and new
access console doesn't have any other place where to revoke this.
Yes, would have to be here
The reason why I suggest to list also all clients from group (b) is
some
potential usability concern. For example assume you have client, which has
active offline token, but it hasn't any roles (for example because this
client doesn't use RBAC). Now what will happen is:
- User clicks "Revoke" on client.
- Client will disappear from the new account console because user doesn't
have any roles for this client and this client doesn't have active offline
token now.
My question is, isn't it confusing from UX perspective that some clients
will disappear from the UI when you click "Revoke" button? Just some
clients will disappear, because clients with any permission/role available
won't disappear.
Or is it an option that clients won't disappear right-away after click
"Revoke", but after page refresh? This would mean that after click
"Revoke"
button, UI can't send another REST request to obtain fresh list of clients
(as that would cause client to disappear).
That is the expected behaviour, and would not be confusing. Try doing it to
GitHub, Google, etc. once you have removed access/consent the client is
removed from the list.
I like the idea of using (user has permission for at least one role with
any client scope) instead of (user has one role) as front-end clients like
SPA type apps won't use any client roles, and it also works when realm
roles are used.
Yes, I agree regarding frontend clients.
Besides that, one of the original reasons for the condition (user has
permission for at least one role with any client scope) is, that it matches
clients with the role scope to "offline_access" role when
"offline_access"
client scope is used. Some time ago, we discussed removing "offline_access"
role. This will makes sense now when we have client scopes and
"offline_access" client scope.
But until "offline_access" role is removed, almost all clients in the
realm will be displayed if we use that condition. I am not sure if this is
what we want or not (Depends on what we agree regarding the UX concern I
had in previous paragraph).
I'm concerned with the approach you (Marek) listed with regards to client
scope. Iterating through every client and calling TokenManager.getAccess is
going to be incredibly expensive, so is not an option, even with
pagination. If you do that with pagination we'd need to fetch 10 clients,
run TokenManager.getAccess, find 1 client with access, then continue until
we've built enough for a single page. It has to be something that we can
actually query directly somehow, but that is difficult with groups and
composite roles.
The performance of TokenManager.getAccess is possibly not so bad. I did
some improvement in this part last year during work on client scopes. Most
of the things don't need DB query as all entities (clients, client scopes,
groups, roles, users) are cached together with their "direct" role
memberships in the local infinispan cache. However for deployments with
thousands of roles or clients, it could be tricky...
What about if there are 10K clients?
Thing is, that AFAIK nobody yet (surprisingly) reported any
performance
issue with the "Applications" tab in the old account console even if it
doesn't have pagination. Maybe it is because people don't use old account
console :) But who knows...
Stan suggested to wait for feedback before doing pagination. I agree with
that.
Marek
On Mon, 7 Oct 2019 at 21:51, Marek Posolda <mposolda(a)redhat.com> wrote:
> On 07. 10. 19 18:09, Stian Thorgersen wrote:
>
> Marek -
>
> One big difference between the new and the old console is that the old
> console only listed applications the user was currently logged-in to
> (basically it was listed in a session, offline or regular). The new console
> also lists applications that are available to the user to log-in to.
>
> No, the old account console doesn't list only applications the user is
> currently logged-in to. It also lists all the applications available to the
> user.
>
> The old account console basically shows all the clients, which matches
> this pseudo-condition:
>
> (client is NOT bearer-only && (client has consent required || (user has
> permission for at least one role with any client scope)))
>
> The last sub-condition is a bit tricky, but simply said, all the clients,
> which are allowed to retrieve offline token are listed in the old console.
> Which are defacto almost all clients, which are not bearer-only.
>
> My point is, that new account console doesn't have any separate page to
> manage offline tokens, is it correct? So the "Applications" page of new
> account console will be still used to revoke offline tokens and consents,
> right? In that case, the new account console should display all the
> clients, for which user can obtain consent or offline token. And offline
> token can by default be retrieved for almost every client in the realm,
> which is not bearer-only. Which would mean that filtering won't help to
> filter too much clients. Hence I guess pagination might be probably needed.
>
> Marek
>
>
> The new console should list applications within a session in the same way
> as it is done in the old console - although not sure removing bearer-only
> is correct. For regular sessions only apps that can do a login is
> registered in the session, for offline sessions the client should be listed
> regardless of its type.
>
> What we've been discussing here is what is the list of applications
> available to a user, but that are not part of the session. What you are
> suggesting doesn't make all that much sense to me in this context.
>
>
>
> On Mon, 7 Oct 2019 at 16:24, Bruno Oliveira <bruno(a)abstractj.org> wrote:
>
>> I just talked with Stian this morning and we agreed on:
>>
>> 1. It's mandatory that Option 1 becomes part version of the New
>> Account Console. The current Jira was updated
>>
https://issues.jboss.org/browse/KEYCLOAK-5628 to reflect such
>> requirement.
>>
>> 2 Filtering and pagination can be postponed for future releases. Jiras
>> to follow up on this are here:
>> -
https://issues.jboss.org/browse/KEYCLOAK-11534
>> -
https://issues.jboss.org/browse/KEYCLOAK-11677
>>
>> If we are all aboard with this, I think we should move on. Otherwise,
>> please let us know.
>>
>> On Fri, Oct 4, 2019 at 1:35 PM Marek Posolda <mposolda(a)redhat.com>
>> wrote:
>> >
>> > On 04. 10. 19 16:41, Stan Silvert wrote:
>> > > On 10/4/2019 10:16 AM, Stian Thorgersen wrote:
>> > >> Okay, so I've re-read and we're on the same page I believe.
Sorry
>> for
>> > >> that (trying to do to many things with too little time).
>> > >>
>> > >> Option 1 limiting the list to real apps/UIs and those the user has
>> > >> access to is what we should do since you are on board with this.
>> > >> Option 2 can then be dropped completely as it was just a quicker
>> > >> temporary solution.
>> > >>
>> > >> To limit to real apps in addition to what I listed before I would
>> also
>> > >> only include apps that have a display name set.
>> > > Ideally, we should have a flag for this. I don't like the idea
that
>> we
>> > > have to rely on the administrator to understand that a display name
>> > > being blank in admin console conveys a certain meaning in account
>> console.
>> > >> To limit apps that users have access to. Thinking about this some
>> more
>> > >> and the ideal I think would be to only list apps where user has at
>> > >> least one client role. That may be a bit tricky though, but
perhaps
>> a
>> > >> smart query could solve that? I'm open to other ideas here for
sure
>> > >> though.
>> > > I think an approach like that would work. It would be helpful to an
>> > > admin if there was something in the admin console that did this query
>> > > and showed explicitly which applications a given user has access to.
>> >
>> > BTV. Some similar filtering is already done in the old account console.
>> >
>> > It filtered the "bearerOnly" clients, but it didn't filter
clients
>> > without baseURL . I think that baseUrl is not mandatory field for
>> > clients and IMO many clients don't have it configured, so not sure
>> > whether to filter based on that...
>> >
>> > In addition to that you need always display clients with offline-access
>> > and with granted consent. The old account console allowed on the
>> > "Applications" page to see and revoke granted consents of clients
and
>> it
>> > also allowed to see and revoke granted offline tokens. So if new
>> account
>> > console doesn't have any other place to view/revoke the consents and
>> > offline tokens, it should be provided on this page.
>> >
>> > However if you filter to see just clients with any client role +
>> clients
>> > with offline-access and granted consent, it may create interesting
>> > situations. For example imagine there is client, which doesn't have any
>> > client roles, but it has consent granted or offline token granted. Now
>> > user clicks the "revoke consent" (or "revoke offline
token") button.
>> > This will cause that client will disappear from the UI because it
>> > doesn't have any client roles and it doesn't have any consent or
>> offline
>> > access. This seems to me like quite confusing behaviour regarding UX?
>> > Also it will affect pagination results etc...
>> >
>> > With regards to this, I wonder if filtering shouldn't be the same as it
>> > was in old account console? This was that client with consentRequired
>> > were always included and clients with ANY role in the token for any
>> > client scope were always included. The details are here:
>> >
>>
https://github.com/keycloak/keycloak/blob/master/services/src/main/java/o...
>> >
>> > It is quite complex to compute if client has permission to see any
>> > single role. You need to make composite roles into account etc. Hence
>> > there is call to TokenManager.getAccess . The performance of this is
>> not
>> > very great, however if you have pagination with showing only 10 clients
>> > per page, it should be just fine to use this IMO.
>> >
>> > In shortcut: I suggest to use exactly same filtering as done by old
>> > account console. but add pagination support to it (which wasn't
>> provided
>> > by old account console). Or alternatively, if new account console has
>> > separate page to manage offline tokens (which it maybe should have?)
>> > then filtering can be done to display clients that:
>> >
>> > are NOT bearerOnly && (have consentRequired OR have any client role
>> > available).
>> >
>> > By "client role available", you may still need to consider
composite
>> > roles, all possible client scopes etc, so the call to
>> > "TokenManager.getAccess" will be still needed.
>> >
>> > Marek
>> >
>> > >
>> > >> On Fri, 4 Oct 2019, 16:10 Stian Thorgersen,
<sthorger(a)redhat.com
>> > >> <mailto:sthorger@redhat.com>> wrote:
>> > >>
>> > >> My bad. I was thinking about comment 1, 2 and 3 from my first
>> reply.
>> > >>
>> > >> Let me re-read the whole thing again ;)
>> > >>
>> > >> On Fri, 4 Oct 2019, 15:42 Bruno Oliveira,
<bruno(a)abstractj.org
>> > >> <mailto:bruno@abstractj.org>> wrote:
>> > >>
>> > >> My comments were pretty much based on the items you
>> mentioned:
>> > >>
>> > >> > 1) Limit the list to clients that are applications
and
>> that
>> > >> the user has access to (I suggested a fairly simple
>> approach,
>> > >> which I believe should work)
>> > >>
>> > >> That wouldn't list the clients regardless if the user
has
>> > >> access to
>> > >> them or not. So I'm not sure where the security issue
is.
>> > >> Unless I'm
>> > >> missing something.
>> > >>
>> > >> > 2) Only list clients from active sessions - then add
a
>> > >> follow-up for 1
>> > >> at some point in the future
>> > >> Yes, that's possible, but as you mentioned something
to
>> postpone
>> > >> unless badly needed. If we keep increasing the scope of
>> what
>> > >> we aim,
>> > >> this may become an endless task.
>> > >>
>> > >> So here are my questions:
>> > >> - Are we in agreement that #1 should be part of our
>> > >> deliverable for
>> > >> the first release of the new account console and #2
>> > >> implemented later?
>> > >> - If yes, are we ok about postponing
pagination/filtering?
>> > >>
>> > >>
>> > >> On Fri, Oct 4, 2019 at 10:24 AM Stian Thorgersen
>> > >> <sthorger(a)redhat.com
<mailto:sthorger@redhat.com>> wrote:
>> > >> >
>> > >> > We're not on the same page. #2 is absolutely not
>> redundant
>> > >> with #1. It is both a security issue and a usability
issue
>> to
>> > >> list all applications regardless if the user has access
to
>> > >> them or not.
>> > >> >
>> > >> > One more not devices page should not list
applications
>> with
>> > >> offline access (offline sessions) those should be on app
>> page
>> > >> (or a separate place?!?)
>> > >> >
>> > >> > On Fri, 4 Oct 2019, 14:49 Bruno Oliveira,
>> > >> <bruno(a)abstractj.org
<mailto:bruno@abstractj.org>> wrote:
>> > >> >>
>> > >> >> I believe that we're all in agreement that we
don't need
>> > >> pagination
>> > >> >> for the Applications endpoint.
>> > >> >>
>> > >> >> And I have the same impression as Stan, #1 makes
perfect
>> > >> sense and
>> > >> >> once it's done should make #2 redundant. If
we are on
>> the
>> > >> same page
>> > >> >> about this, I can update
>> > >> >>
https://issues.jboss.org/browse/KEYCLOAK-5628.
>> > >> >>
>> > >> >> Another question is: assuming that we implement
#1. Do
>> we
>> > >> still need
>> > >> >> filtering (
>>
https://issues.jboss.org/browse/KEYCLOAK-11534)?
>> > >> >>
>> > >> >>
>> > >> >> On Fri, Oct 4, 2019 at 8:59 AM Stian Thorgersen
>> > >> <sthorger(a)redhat.com
<mailto:sthorger@redhat.com>> wrote:
>> > >> >> >
>> > >> >> > You can not have an application page in the
new
>> account
>> > >> console that lists every client there is in a realm. As I
>> said
>> > >> a large portion of those will not be actual applications,
>> and
>> > >> a portion will be applications that the user does not
have
>> > >> access to.
>> > >> >> >
>> > >> >> > There's really two choices here:
>> > >> >> >
>> > >> >> > 1) Limit the list to clients that are
actually
>> > >> applications and that the user has access to (I suggested
a
>> > >> fairly simple approach, which I believe should work)
>> > >> >> > 2) Only list clients from active sessions -
then add a
>> > >> follow-up for 1 at some point in the future
>> > >> >> >
>> > >> >> > My preference here would be 1 for sure as if
this is
>> done
>> > >> right it would be a good value add for users to have a
>> place
>> > >> to discover available applications.
>> > >> >> >
>> > >> >> > On Thu, 3 Oct 2019 at 11:54, Bruno Oliveira
>> > >> <bruno(a)abstractj.org
<mailto:bruno@abstractj.org>> wrote:
>> > >> >> >>
>> > >> >> >> On 2019-10-03, Stian Thorgersen wrote:
>> > >> >> >> > Simply returning all clients is not
going to work
>> for
>> > >> a few reasons:
>> > >> >> >> >
>> > >> >> >> > * It will return clients that are
not
>> applications/UIs
>> > >> >> >> > * It can return applications the
user doesn't have
>> > >> access to
>> > >> >> >> > * There can be thousands (in fact
we know about
>> users
>> > >> with 10K+ clients)
>> > >> >> >> >
>> > >> >> >> > That means we need the following:
>> > >> >> >> >
>> > >> >> >> > 1) Limit clients returned by the
REST endpoint to
>> only
>> > >> those that are
>> > >> >> >> > indeed applications/UIs
>> > >> >> >>
>> > >> >> >> That makes sense, at the same time, not
part of our
>> > >> requirements into the
>> > >> >> >> Jira:
https://issues.jboss.org/browse/KEYCLOAK-5628.
>> > >> >> >>
>> > >> >> >> Doug is working on it, and if
there's anything that
>> has
>> > >> to change, I'd
>> > >> >> >> suggest we bring this up in the same
Jira.
>> > >> >> >>
>> > >> >> >> > 2) Limit applications to those the
user has access
>> to
>> > >> >> >>
>> > >> >> >> Same as my previous comment
>> > >> >> >>
>> > >> >> >> > 3) Support filtering and pagination
(even though 1
>> and
>> > >> 2 most likely will
>> > >> >> >> > significantly reduce the number of
applications to
>> 10s
>> > >> of applications, we
>> > >> >> >> > still need to have pagination and
filtering
>> support)
>> > >> >> >>
>> > >> >> >> We have a Jira for filtering, but not
for pagination.
>> > >> >> >> See:
https://issues.jboss.org/browse/KEYCLOAK-11534.
>> But
>> > >> if you think
>> > >> >> >> pagination should also be a part of it,
please let us
>> > >> know. Just keep in
>> > >> >> >> mind that this is not part of our plans
at the
>> moment.
>> > >> >> >>
>> > >> >> >> Do you really think we need to implement
pagination
>> for
>> > >> Applications
>> > >> >> >> endpoint right now? Based on the
requirements you
>> > >> described, I don't see
>> > >> >> >> a user with 2000 applications. Just look
at how many
>> > >> applications you
>> > >> >> >> have linked into your GH or FB profile.
>> > >> >> >>
>> > >> >> >> Maybe this is something we could
postpone? Unless I'm
>> > >> missing something,
>> > >> >> >> I don't see a real need to do it
right now.
>> > >> >> >
>> > >> >> >
>> > >> >> > If you do 1 or 2 the list of applications
available to
>> > >> any given user will be reduced significantly, so I'm
fairly
>> > >> confident that pagination/filtering on the server-side
can
>> be
>> > >> postponed in that case.
>> > >> >> >
>> > >> >> >>
>> > >> >> >>
>> > >> >> >> >
>> > >> >> >> > Some ideas on how we can achieve
the above:
>> > >> >> >> >
>> > >> >> >> > 1) Figuring out what is indeed
applications/UIs
>> > >> >> >> >
>> > >> >> >> > List applications that are added to
open sessions,
>> > >> including the below:
>> > >> >> >> >
>> > >> >> >> > * All OIDC clients where:
client.baseUrl != null &&
>> > >> !client.bearerOnly
>> > >> >> >> > * All SAML clients where:
client.baseUrl != null**
>> > >> >> >> >
>> > >> >> >> > This will make sure we only include
applications
>> where
>> > >> the user can
>> > >> >> >> > actually click on the application
in the list to
>> go to
>> > >> the application.
>> > >> >> >> >
>> > >> >> >> > ** Not sure if there's anything
in addition to
>> check
>> > >> for SAML
>> > >> >> >> >
>> > >> >> >> > 2) Limit applications to those the
user has access
>> to
>> > >> >> >> >
>> > >> >> >> > Not sure about this one as we
don't really have an
>> > >> easy way to figure out
>> > >> >> >> > if a user has access the an
application or not. One
>> > >> idea would be to only
>> > >> >> >> > include clients where user has at
least one client
>> > >> role. Even if the
>> > >> >> >> > application doesn't use client
roles directly a
>> > >> "dummy" role can be created
>> > >> >> >> > for this purpose by
admins/developers.
>> > >> >> >> >
>> > >> >> >> > 3) Pagination and filtering
>> > >> >> >> >
>> > >> >> >> > All endpoints should support
pagination and
>> filtering
>> > >> by design. Pagination
>> > >> >> >> > and filtering should be server-side
(REST endpoint
>> > >> should provide according
>> > >> >> >> > to our REST guidelines).
>> > >> >> >>
>> > >> >> >> +1 for most of the ideas, except for
implementing
>> > >> pagination right now.
>> > >> >> >>
>> > >> >> >> >
>> > >> >> >> > On Wed, 2 Oct 2019 at 19:11, Stan
Silvert
>> > >> <ssilvert(a)redhat.com
<mailto:ssilvert@redhat.com>> wrote:
>> > >> >> >> >
>> > >> >> >> > > Specifically, we need to
discuss filtering and
>> > >> pagination as it relates
>> > >> >> >> > > to the
"Applications" page:
>> > >> >> >> > >
>> > >> >> >> > >
https://marvelapp.com/c90dfi0/screen/59942290
>> > >> >> >> > >
>> > >> >> >> > > The current design allows
filtering by name and
>> > >> application type.
>> > >> >> >> > >
>> > >> >> >> > > However, Stian has pointed out
that some
>> customers
>> > >> will have thousands
>> > >> >> >> > > of clients. So this design
might be unworkable.
>> > >> >> >> > >
>> > >> >> >> > > I don't want to go too far
into the weeds right
>> now
>> > >> because I want to
>> > >> >> >> > > understand the problem better
first.
>> > >> >> >> > >
>> > >> >> >> > > What is the use case when
customers have many,
>> many
>> > >> clients?
>> > >> >> >> > >
>> > >> >> >> > > How common is it to have many,
many clients for a
>> > >> single user?
>> > >> >> >> > >
>> > >> >> >> > > What do those clients look
like?
>> > >> >> >> > >
>> > >> >> >> > > What could we use to filter
on? The information
>> we
>> > >> currently have on
>> > >> >> >> > > the client side looks
something like what you
>> see here:
>> > >> >> >> > >
>> > >> >> >> > >
https://marvelapp.com/c90dfi0/screen/59942292
>> > >> >> >> > >
>> > >> >> >> > >
_______________________________________________
>> > >> >> >> > > keycloak-dev mailing list
>> > >> >> >> > > keycloak-dev(a)lists.jboss.org
>> > >> <mailto:keycloak-dev@lists.jboss.org>
>> > >> >> >> > >
>>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> > >> >> >> >
_______________________________________________
>> > >> >> >> > keycloak-dev mailing list
>> > >> >> >> > keycloak-dev(a)lists.jboss.org
>> > >> <mailto:keycloak-dev@lists.jboss.org>
>> > >> >> >> >
>>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> > >> >> >>
>> > >> >> >> --
>> > >> >> >>
>> > >> >> >> abstractj
>> > >> >>
>> > >> >>
>> > >> >>
>> > >> >> --
>> > >> >> - abstractj
>> > >>
>> > >>
>> > >>
>> > >> --
>> > >> - abstractj
>> > >>
>> > > _______________________________________________
>> > > keycloak-dev mailing list
>> > > keycloak-dev(a)lists.jboss.org
>> > >
https://lists.jboss.org/mailman/listinfo/keycloak-dev
>> >
>> >
>>
>>
>> --
>> - abstractj
>>
>
>