Unicon / cas-addons

Open source CAS customizations, extensions, and configuration aids.
http://unicon.github.io/cas-addons/
Apache License 2.0
54 stars 26 forks source link

Modify ServiceAuthorizationAction.java to handle services without RBAC rules in the service registry #21

Closed epierce closed 11 years ago

epierce commented 11 years ago

I'd like to use the RBAC feature, but I didn't want to put rules on all 150+ registered services, so I added a check to see if the response from getExtraAttributes().get(AUTHZ_ATTRS_KEY) was null or not. If it's null (no RBAC rules), the rule processing is skipped and the webflow execution continues. I also added some code to handle the services that are disabled or were not found in the service registry.

BTW, this was the only file in the project with CRLF line endings so I changed that as well.

apetro commented 11 years ago

+1 for thoughtfully enhanced RBAC functionality. Much appreciated, @epierce .

I'd like to see this have a unit test before it's merged, since part of what this is proving is that there's some interesting edge-ish cases to be carefully attended. If you want to add that testng, @epierce , great; if not I'll help Unicon to do that before merge.

epierce commented 11 years ago

I'll take a shot at it. I haven't done unit tests for a SWF action before, but I should be able find an example in the CAS code to follow.

dima767 commented 11 years ago

Appreciate the enhancement as well! — Sent from Mailbox for iPhone

On Wed, Jun 19, 2013 at 12:59 PM, Andrew Petro notifications@github.com wrote:

+1 for thoughtfully enhanced RBAC functionality. Much appreciated, @epierce .

I'd like to see this have a unit test before it's merged, since part of what this is proving is that there's some interesting edge-ish cases to be carefully attended. If you want to add that testng, @epierce , great; if not I'll help Unicon to do that before merge.

Reply to this email directly or view it on GitHub: https://github.com/Unicon/cas-addons/pull/21#issuecomment-19697715

apetro commented 11 years ago

@epierce, @mmoayyed has been doing some unit tests for the web flow actions in https://github.com/Unicon/cas-mfa/tree/master/cas-mfa-web/src/test/java/net/unicon/cas/mfa/web/flow ; you might take a look at those as an input.

epierce commented 11 years ago

Added unit tests for ServiceAuthorizationAction and DefaultRegisteredServiceAuthorizer. The test for DefaultAuthenticationSupport that @dima767 wrote was a huge help, thanks.

dima767 commented 11 years ago

Wonderful. Will merge tomorrow. 

​Thanks, D. 

— Sent from Mailbox for iPhone

On Thu, Jun 20, 2013 at 8:03 PM, Eric Pierce notifications@github.com wrote:

Added unit tests for ServiceAuthorizationAction and DefaultRegisteredServiceAuthorizer. The test for DefaultAuthenticationSupport that @dima767 wrote was a huge help, thanks.

Reply to this email directly or view it on GitHub: https://github.com/Unicon/cas-addons/pull/21#issuecomment-19791607