New oidc fixes

[mrvanes]

As requested on the idpy meeting, I hereby created a new pr containing separate commits for each sub-fix, a config option for oidc multi_value and an accompanying test for oidc multi_value. I hope it lives up to your expectations. There is also a whitespace fix, because my editor is set to remove trailing whitespace stuff.

[c00kiemon5ter]

This is a bit complex. I'm adding whitespace to make it readable

assert all(
  (k, v) in {
    ck: cv if isinstance(cv, list) else [cv]
    for ck, cv in id_token_claims.items()
  }.items()
  for k, v in USERS[user_id].items())

What you're trying to do is figuring out if all items from USERS match an item in id_token_claims after a transformation. This means that USERS is a subset of (the transformed) id_token_claims.

claims_dict =  {
    ck: cv if isinstance(cv, list) else [cv]
    for ck, cv in id_token_claims.items()
}
claims_set = set(claims_dict.items())
users_set = set(USERS[user_id].items())
assert users_set.issubset(claims_set)

I think, though, that what you're really after is equality of the dicts.

claims_dict =  {
    ck: cv if isinstance(cv, list) else [cv]
    for ck, cv in id_token_claims.items()
}
assert claims_dict == USERS[user_id]

[mrvanes]

I see a lot of good comments and suggestions, most of which because you're a better python hacker. What's your idea about moving forward? Our tree is way out of sync, so making a new PR would require substantial effort while the gist of the fixes is clear (imo)?

[c00kiemon5ter]

@mrvanes I'm going through this PR, but I'm not sure how this is useful. This is some test data* for authn_req["claims"]:

authn_req["claims"] = {
  "id_token": {
    "foo": "bar",
    "mail": {
      "essential": True
    }
  },
  "userinfo": {
    "acr": {
      "essential": True,
      "values": [
        "urn:mace:incommon:iap:silver",
        "urn:mace:incommon:iap:bronze"
      ]
    }
  }
}

Coming out of the process, the result is:

> print(requested_claims)
['sub', 'foo', 'email.essential', 'acr.essential', 'acr.values']

Do we really want email.essential or acr.values? How is it more correct than before?

*link to the relevant section in the spec