jbittel / django-mama-cas

A Django Central Authentication Service (CAS) single sign-on server
BSD 3-Clause "New" or "Revised" License
396 stars 112 forks source link

Handle user attributes of type list #74

Closed tarunpaul closed 5 years ago

tarunpaul commented 5 years ago

The ValidationResponse should handle list attributes.

https://apereo.github.io/cas/4.2.x/protocol/CAS-Protocol-Specification.html#257-example-response-with-custom-attributes

{
  "serviceResponse" : {
    "authenticationSuccess" : {
      "user" : "username",
      "proxyGrantingTicket" : "PGTIOU-84678-8a9d...",
      "proxies" : [ "https://proxy1/pgtUrl", "https://proxy2/pgtUrl" ],
      "attributes" : {
        "firstName" : "John",
       "affiliation" : [ "staff", "faculty" ]
        "title" : "Mr.",
        "email" : "jdoe@example.orgmailto:jdoe@example.org",
        "lastname" : "Doe"
      }
    }
  }
}
  <cas:serviceResponse xmlns:cas="http://www.yale.edu/tp/cas">
    <cas:authenticationSuccess>
      <cas:user>username</cas:user>
      <cas:attributes>
        <cas:firstname>John</cas:firstname>
        <cas:lastname>Doe</cas:lastname>
        <cas:title>Mr.</cas:title>
        <cas:email>jdoe@example.org</cas:email>
        <cas:affiliation>staff</cas:affiliation>
        <cas:affiliation>faculty</cas:affiliation>
      </cas:attributes>
      <cas:proxyGrantingTicket>PGTIOU-84678-8a9d...</cas:proxyGrantingTicket>
    </cas:authenticationSuccess>
  </cas:serviceResponse>

"affiliation" : [ "staff", "faculty" ] Should translate to

<cas:affiliation>staff</cas:affiliation>
<cas:affiliation>faculty</cas:affiliation>
tarunpaul commented 5 years ago

I have created a pull request https://github.com/jbittel/django-mama-cas/pull/75

Please let me know if this looks ok.

manelclos commented 5 years ago

@tarunpaul thanks! I've taken a quick look and the PR is missing a test. I'd say that the for loop you created overwrites the attribute.text with the last value of the list. A test would prove me wrong ;)

Could you please add the test?

tarunpaul commented 5 years ago

@manelclos I have added a test case. Please take a look. And thank you for keeping up the code standards. Really appreciate it. :)

tarunpaul commented 5 years ago

Hi @manelclos, Please let me know if this looks ok or if I have missed anything. Few test environments seem to fail on travis-ci. Two of which are unable to connect to internet to download the specific python version. But locally it seems to pass all test cases.

$ python3 runtest.py Creating test database for alias 'default'... Ran 157 tests in 1.767s OK Destroying test database for alias 'default'...

manelclos commented 5 years ago

@tarunpaul just checked the code locally, all good! Let's see if @jbittel has some time to approve it.

tarunpaul commented 5 years ago

Great! Thank you @manelclos :)

jbittel commented 5 years ago

Merged. Much appreciated!