Let cas_server module send attributes

Agreed. Patches are welcome. It looks like I need to do a sprint on the CAS server module soon. I'll work on this when I do.

A lot of the code needed to send out these attributes could be ripped out of the cas_test module, which should save some time.

This seems like a great idea when CAS clients redirect various user types to the appropriate home pages.

Implementing this feature should also handle my issue #1185772: Use Drupal roles returned by CAS Server unless there is an easier way I've missed?

Attaching a starter patch which moves some theme functions from cas_test to cas_server. This won't be functional yet, but I want to make sure I don't break the existing CAS tests.

Great, I've committed #4 to both 6.x-3.x and 7.x-1.x. Resetting the issue status since more work still needs to be done.

Now that we have themed responses in CAS Server, we can use these to process the serviceValidate requests.

Once nice thing is the response themes (see cas_server.response.inc) already have attribute support. So, by passing in an appropriate array we can automatically return these attributes. The attached patches use the theme functions (but don't pass any attributes yet.

The big design question here is how do we determine which attributes, if any, to return? Perhaps this calls for another API definition -- say hook_cas_server_user_attributes() --- which returns an associative array of attributes to be returned to the CAS server?

/**
 * @file
 * Documentation for CAS Server API.
 */

/**
 * Return additional CAS attributes when acting as a CAS server.
 *
 * This hook allows modules to add additional CAS attributes to the basic
 * response by the CAS Server module.
 *
 * @param $account
 *   The user being logged in.
 * @param $service
 *   The service URL of the site the user is logging in to.
 *
 * @return
 *   An associative array of CAS attributes for the user.
 */
function hook_cas_server_user_attributes($account, $service) {
  return array(
    'key1' => 'single_value',
    'key2' => array('multiple', 'values'),
    'drupal_roles' => $account->roles,
  );
}

Please review the attached patch for Drupal 6. This patch:

(1) Provides an API for other modules to return CAS attributes.
(2) By default returns the user's (Drupal) roles as an array of role names.
(3) Seems to not break the basic functionality (in my limited testing).

subscribe

Maybe I'm missing something, but why does the latest patch include roles as an attribute but not the email address?

We might as well expose all of the basic user metadata. I'll see if I can find some time to write some code to let users choose what fields attached to the user bundle should be exposed as CAS attributes. This won't catch things from the profile2 module, but will at least catch most of the basics.

Also, it looks like it would make sense to convert _cas_server_validate() to either return a full user object. It would keep the code flow cleaner since we need to pass it on in the API anyways.

While I'm on the subject, cas_server_service_validate() tries to use $user_name in the watchdog statement on failure. Since failure is detected by the lack of a username, I assume this is a bug.

Maybe I'm missing something, but why does the latest patch include roles as an attribute but not the email address?

Sounds like a great idea to me. I have no use for this at the moment which is why it didn't occur to me.

We might as well expose all of the basic user metadata. I'll see if I can find some time to write some code to let users choose what fields attached to the user bundle should be exposed as CAS attributes. This won't catch things from the profile2 module, but will at least catch most of the basics.

Do we need to worry about visibility permissions (i.e., which fields should be shared and which shouldn't?).

While I'm on the subject, cas_server_service_validate() tries to use $user_name in the watchdog statement on failure. Since failure is detected by the lack of a username, I assume this is a bug.

I created a new issue for it: #1259926: cas_server_service_validate() uses $user_name when it isn't set.

Do we need to worry about visibility permissions (i.e., which fields should be shared and which shouldn't?).

Since we don't do any filtering of the service coming in, it seems like it could be prudent. On the other hand, the attributes aren't shared unless auth succeeds. I see this as being akin to setting what 3rd party sites see when using your Facebook credentials to login.

Another approach would be to implement the functionality that limits ticket validation requests to a list of URLs. This is the way that the CAS server typically handles it. The username itself is considered an attribute. So if you allowed the user to whitelist the URLS that it can accept service requests to, you could control whether the attributes got sent outside that way.

I've created a patch for service whitelisting: #1260588: CAS Server service whitelisting

Here is a D7 version of the patch in #7.

Here is a version of the patch in #14, but with the addition of some additional user attributes being exported.

Patch doesn't apply since #1259926: cas_server_service_validate() uses $user_name when it isn't set. was committed.

Since we don't have any tests for the CAS Server yet, this will need some manual testing before I'll commit it.

Changes from #16:
- Whitespace fixes.
- Add drupal_alter('cas_server_user_attributes') call after module_invoke_all call(...) to keep with the Drupal pattern.
- Add $ticket as a parameter to hook_cas_server_user_attributes().

*throwing my hands up in the air!*

From #1399570: Get CAS-Server user ID from CAS-Client, we probably also want to include the Drupal user uid.

subscribe

I'm using the latest patch above (soon in production) and have implemented an extension to provide my own attributes. Is it safe to bump this to 'reviewed & tested'?

Committed to 7.x-1.x. Needs to be ported to 6.x-3.x.

Committed to 6.x-3.x.

Thanks a lot for the API

+++ b/cas_server.api.phpundefined
@@ -0,0 +1,50 @@
+  // Or change the response based upon the server.
+  if (preg_match($service, "@^http://apple.com/@")) {

This confused me when implementing it.
The pattern comes first in preg_match...
so it should be:

if (preg_match("@^http://apple.com/@", $service)) {

Thanks for patch. Next time make a new issue -- I originally missed the fact that you had re-opened this one.

Committed to 6.x-3.x and 7.x-1.x.