On 20.4.2015 16:29, Stian Thorgersen wrote:
----- Original Message -----
> From: "Marek Posolda" <mposolda(a)redhat.com>
> To: "Stian Thorgersen" <stian(a)redhat.com>
> Cc: keycloak-dev(a)lists.jboss.org
> Sent: Monday, April 20, 2015 4:04:53 PM
> Subject: Re: [keycloak-dev] Persistent grants - step 1
>
> On 20.4.2015 07:36, Stian Thorgersen wrote:
>> ----- Original Message -----
>>> From: "Marek Posolda" <mposolda(a)redhat.com>
>>> To: keycloak-dev(a)lists.jboss.org
>>> Sent: Friday, April 17, 2015 2:32:30 PM
>>> Subject: [keycloak-dev] Persistent grants - step 1
>>>
>>> I've sent startup PR for persistent grants. I've added
>>> GrantedConsentModel (is it good name?) to track the roles and protocol
>>> mappers user granted on consent screen. Consent screen is not displayed
>>> if user already approved roles and protocol mappers before. There is
>>> also new "Access" tab in account management where can user see
>>> previously granted consents and revoke them for particular client. Is
>>> "Access" good name for the tab?
>> UserConsentModel?
> That's better indeed:-)
> I will rename it.
>
>> I'd rather call the page in account management 'Applications' tab.
One
>> thing I'd like us to add is a list of applications a user can access. It
>> makes more sense to me to have a single place to view everything related
>> to applications (which are available, what access do they have, what
>> sessions are they logged-in to, etc..) than having multiple pages.
> Tricky is what exactly means "which applications are available" .
> Currently user can retrieve accessToken for any "confidential" or
> "public" application in the realm. And even if user doesn't have any
> roles or scopes (access token would have both fields realmAccess and
> resourceAccess empty), the access token could be still usable though.
>
> For example our ServerInfoAdminResource endpoint currently allows to be
> authenticated with any accessToken for any client and it doesn't require
> any roles available (which is probably not correct behaviour I guess,
> but that's another issue though...)
You're focusing to much on a corner-case. A client that has a base-url and has a role
or scope on at least one of the users roles is sufficient. If that doesn't work for a
"special" app it can be solved by simply adding a fictitious role.
yeah,
focusing on corner case is maybe one of my issues :-)
I will do the way you mentioned.
ServerInfoAdminResource should probably require at least one admin role to be present,
can you jira?
https://issues.jboss.org/browse/KEYCLOAK-1218
>>> There is still some work and I will continue if you don't have any
>>> comments to the current impl. Remaining TODO is:
>>> - representations
>>> - Mongo model (for now I have just JPA. I hope that "file" model
could
>>> be skipped for now?)
>>> - More automated tests
>>>
>>> Some additional things to discuss:
>>> - I would like to add
>>> ClientSessionModel.setProtocolMappers/getProtocolMappers (Set<String>
>>> with Ids of protocolMappers similarly like current getRoles/setRoles).
>>> There is one small issue that protocolMappers are actually always
>>> computed from the ClientModel, which means that there could be
>>> theoreticallly different protocolMappers displayed to user and then
>>> later saved to persistent model. Also later we want to add support for
>>> "scope" parameter, which means that protocolMappers on the consent
>>> screen could be really different than protocolMappers from the
>>> ClientModel. Any objections against it?
>>>
>>> - It may be good to add ClientModel.setDescription/getDescription, so on
>>> the consent screen and in Account management Access page is displayed
>>> the more proper (and localized) description of the client instead of
>>> clientId (ie. "Have User Privileges in ThirdParty Application"
instead
>>> of "Have User Privileges in third-party"). Any objections against
it?
>> +1 To add, but call it ClientModel.get/setName instead. This makes me think
>> that we need a way to manage localized labels through the console. If
>> someone sets name of a client to ${myClient} then they should also be able
>> to add translations to it through the admin console. That's something to
>> implement later though.
> hmm... Actually on RoleModel, there is "setDescription/getDescription"
> used as localized field, so I wonder if ClientModel shouldn't be
> consistent with that?
I disagree. For a role you want to "describe" to the user what the application
is requesting access to. In this case the name is meaningless. For example it makes more
sense on the consent screen to display "Read your emails" than "Read
Email".
For a app/client it's the name you want to display. For example "ACME Email
Readers" makes more sense then "An application that can read emails". There
may be a case for adding a description as well to a client (for example to display in
account management), but that can be done later if we need to.
ok, makes sense
> I agree with translations of clients, roles and protocol mappers
> available from admin console. I've proposed that too couple of months ago.
Maybe one day we have time to implement it ;)
> Also I wonder if we should have single "messages_*.properties" scoped to
> the whole Keycloak (theme) instead of separate messages.properties file
> per each theme type? Because there are many keys duplicated per both
> "login" and "account" theme type (and there will be even more if
we ever
> add localization for admin console). Classic example are protocol mapper
> or roles descriptions (firstName, lastName, ...)
Same answer as above - it probably makes more sense, but would have to be done later.
Actually it probably makes sense to have message bundles completely detached from themes
altogether.
https://issues.jboss.org/browse/KEYCLOAK-1219
Single file is also more friendly to localization community contributors
IMO. They don't need to translate same things twice and it will help to
avoid the situation when contributor translates just messages for
"login" but miss the "account" or "email".
Marek
> Marek
>>> - There is bit related issue
>>>
https://issues.jboss.org/browse/KEYCLOAK-1216 that click on "Logout all
>>> sessions" in Account management doesn't propagate logout to the
apps.
>>> Currently I invalidate clientSessions of particular client and user
>>> during revoke, but also don't propagate it to the applications. I would
>>> like to change that and propagate it and also fix KEYCLOAK-1216 at the
>>> same time. There will be still small issue for JS applications, because
>>> when just clientSession of JS application is revoked, the logout won't
>>> be propagated to the actual application because KEYCLOAK_SESSION cookie
>>> is still valid. So for JS applications, the application will be really
>>> logged later when accessToken expires. Any objections against it? Any
>>> idea how to propagate revoke to JS applications?
>> Assuming 1216 is the way it's work all along, let's postpone that. Same
>> goes for propagating "consent revocation" to apps, jira it and postpone
to
>> next release.
>>
>>> Thanks,
>>> Marek
>>> _______________________________________________
>>> keycloak-dev mailing list
>>> keycloak-dev(a)lists.jboss.org
>>>
https://lists.jboss.org/mailman/listinfo/keycloak-dev
>>>
>