uPortal IRC Logs-2012-07-13

[12:38:58 CDT(-0500)] <drewwills> EricDalquist? athena? are you there?

[12:39:08 CDT(-0500)] <drewwills> can you take a quick gander at this: https://issues.jasig.org/browse/UP-3515

[12:39:09 CDT(-0500)] <EricDalquist> yup

[12:39:41 CDT(-0500)] <EricDalquist> looks interesting

[12:39:52 CDT(-0500)] <EricDalquist> athena: would be the one to get to review it as she's done most of the rest work to date

[12:40:07 CDT(-0500)] <drewwills> i owe that to SSP today if we can come to consensus and I can get through the coding (which is small)

[12:40:30 CDT(-0500)] <drewwills> she and I talked it over yesterday for a bit

[12:40:42 CDT(-0500)] <drewwills> but this is a writeup... with more specifics

[12:40:58 CDT(-0500)] <EricDalquist> I mean it looks good to me

[12:41:07 CDT(-0500)] <EricDalquist> I'd just want her input on if the url looks good

[12:41:19 CDT(-0500)] <drewwills> yeah me too

[12:41:28 CDT(-0500)] <EricDalquist> I don't think there are any potential security issues with a user getting a list of their permissions

[12:41:32 CDT(-0500)] <athena> i'd at least do assignments/principal/self.json

[12:41:41 CDT(-0500)] <drewwills> ah sure

[12:41:45 CDT(-0500)] <EricDalquist> what if there is a principal named self theough?

[12:41:51 CDT(-0500)] <EricDalquist> seems like a resonable last name

[12:41:51 CDT(-0500)] <athena> yeah, that's a problem

[12:42:02 CDT(-0500)] <EricDalquist> we have a guy down the hall whose username is "terrible"

[12:42:08 CDT(-0500)] <athena> and in some ways it seems like we really want to just use the same URL

[12:42:14 CDT(-0500)] <athena> since it's not like we don't know the username of the current user

[12:42:40 CDT(-0500)] <EricDalquist> oh true I assumed that wasn't available based on this writup

[12:42:44 CDT(-0500)] <athena> and maybe just modify that existing method to first check if you're an admin, then check if the session user matches the requested user

[12:42:49 CDT(-0500)] <EricDalquist> is it not in your case drewwills?

[12:42:51 CDT(-0500)] <drewwills> roger... so we could let you through if you're inquiring about yourself?

[12:42:54 CDT(-0500)] <athena> seems like maybe that's the simplest approach?

[12:42:58 CDT(-0500)] <EricDalquist> oh right

[12:43:01 CDT(-0500)] <EricDalquist> the self-req

[12:43:02 CDT(-0500)] <EricDalquist> yeah

[12:43:04 CDT(-0500)] <athena> and then you don't need to write a new method

[12:43:06 CDT(-0500)] <EricDalquist> that seems reasonable

[12:43:23 CDT(-0500)] <drewwills> yeah that would be reasonable to me

[12:44:19 CDT(-0500)] <drewwills> the existing method has @PreAuthorize("hasPermission('string', 'ALL', new org.jasig.portal.spring.security.evaluator.AuthorizableActivity('UP_PERMISSIONS', 'VIEW_PERMISSIONS'))")

[12:44:55 CDT(-0500)] <athena> you should be able to update that annotation

[12:45:38 CDT(-0500)] <drewwills> what might it look like?

[12:45:47 CDT(-0500)] <athena> could probably turn it into an and that combines the existing logic with a check on whether the param matches the current user

[12:45:48 CDT(-0500)] <drewwills> @PreAuthorize("hasPermission('string', 'ALL', new org.jasig.portal.spring.security.evaluator.AuthorizableActivity('UP_PERMISSIONS', 'VIEW_PERMISSIONS'))" || something_else)

[12:45:58 CDT(-0500)] <athena> yeah

[12:46:10 CDT(-0500)] <athena> take a look at the spring security documentation - there are some nice examples

[12:54:25 CDT(-0500)] <drewwills> lol..."For example, if you wanted a particular method to only allow access to a user whose username matched that of the contact, you could write"

[12:54:34 CDT(-0500)] <drewwills> "@PreAuthorize("#contact.name == authentication.name")"

[12:58:14 CDT(-0500)] <athena> yes

[12:58:20 CDT(-0500)] <athena> their examples are in fact quite helpful

[12:59:52 CDT(-0500)] <drewwills> I'm trying: @PreAuthorize("#ownerParam == authentication.name or hasPermission('string', 'ALL', new org.jasig.portal.spring.security.evaluator.AuthorizableActivity('UP_PERMISSIONS', 'VIEW_PERMISSIONS'))")

[13:21:55 CDT(-0500)] <drewwills> ack! the parameter "@PathVariable("principal") String principal" expects something like "15.student"

[13:22:52 CDT(-0500)]

[13:23:32 CDT(-0500)] <drewwills> (15 == EntityType.id for IPerson)

[13:40:12 CDT(-0500)] <drewwills> bb in a bit

[13:43:17 CDT(-0500)] <athena> yes, that seems reasonable to me

[13:46:46 CDT(-0500)]

[13:47:04 CDT(-0500)] <athena> then you could query either /assignments/user/student.json, or assignments/group/local.2.json

[13:53:46 CDT(-0500)] <EricDalquist> +1 to that

[15:48:25 CDT(-0500)] <drewwills> that would work nicely... i come up w/ another idea while I was gone... checking into it

[15:50:58 CDT(-0500)] <drewwills> i'm (very) good with "/assignments/user/student.json" but it might be less disruptive to do something like "/assignments/principal/IPerson.student.json"

[15:51:23 CDT(-0500)] <drewwills> because if we do the first, we'll have to change the existing stuff that uses it

[15:58:27 CDT(-0500)] <drewwills> where both old & new are supported, such that "/assignments/principal/IPerson.student.json" == "/assignments/principal/15.student.json"

[15:59:26 CDT(-0500)] <EricDalquist> I'm happy with either

[15:59:55 CDT(-0500)] <athena> i think i'd rather not go with that approach

[15:59:58 CDT(-0500)] <EricDalquist> though we don't have a greate "short nane" for EntityType

[16:00:02 CDT(-0500)] <EricDalquist> for either of the approaches

[16:00:04 CDT(-0500)] <EricDalquist> well hrm

[16:00:07 CDT(-0500)] <athena> we do actually

[16:00:11 CDT(-0500)] <athena> and it's being used by other services

[16:00:12 CDT(-0500)] <EricDalquist> oh we do?

[16:00:40 CDT(-0500)] <athena> my recommendation would be to keep /assignments/principal/principalid.json as is

[16:00:52 CDT(-0500)] <EricDalquist> is it part of the actual entity-type data model athena?

[16:00:54 CDT(-0500)] <athena> and add assignments/entityType/id.json

[16:00:57 CDT(-0500)] <EricDalquist> or is it configured somewhere else?

[16:01:02 CDT(-0500)] <athena> it's hard-coded

[16:01:05 CDT(-0500)] <athena> unfortunately.

[16:01:06 CDT(-0500)] <EricDalquist> ah ok

[16:01:15 CDT(-0500)] <athena> part of that JsonEntityBean wrapper stuff

[16:01:16 CDT(-0500)] <EricDalquist> we should get a todo out there to move that into EntityType

[16:01:26 CDT(-0500)] <athena> currently EntityEnum.java

[16:03:01 CDT(-0500)] <EricDalquist> ah

[16:03:03 CDT(-0500)] <EricDalquist> ok

[16:03:08 CDT(-0500)] <athena> supporting both principal ids and typename.id in the same parameter is likely to get messy

[16:03:12 CDT(-0500)] <athena> and be kind of ugly parsing-wise

[16:03:19 CDT(-0500)] <athena> think it's a bit safer to just make those separate

[16:04:26 CDT(-0500)] <EricDalquist> yeah

[16:04:33 CDT(-0500)] <EricDalquist> I forgot other code is using this

[16:04:45 CDT(-0500)] <EricDalquist> in which case it seems safer to have the entity type be part of the path

[16:04:51 CDT(-0500)] <athena> all the code right now is internal and limited, so we could make changes if we need to

[16:04:58 CDT(-0500)] <EricDalquist> you can still do it in a single method with the rest template stuff in spring 3

[16:04:59 CDT(-0500)] <drewwills> so we could pull the impl code into a private method and call it with the existing endpoint, then create a new enpoint after the new URL pattern, juggle some params, and call the same private method

[16:05:09 CDT(-0500)] <athena> but when you do know the principal id it's preferable to use that

[16:06:02 CDT(-0500)] <athena> yeah i think we'd want two methods with two separate param lists / URL paths that both call a shared protected method

[16:10:32 CDT(-0500)]

[16:11:53 CDT(-0500)] <EricDalquist> fyi https://github.com/edalquist/RegexCoach

[16:12:05 CDT(-0500)]

[16:12:07 CDT(-0500)] <EricDalquist> I sat down and cleaned up an old old groovy script I had found online long ago

[16:12:16 CDT(-0500)] <EricDalquist> really useful UI for writing regex

[16:12:21 CDT(-0500)] <athena> oh neat!

[16:12:35 CDT(-0500)] <athena> i'll have to try it out