Skip to content
New issue

Have a question about this project? # for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “#”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? # to your account

Allow AL internal UniqueID to be used as attribute #135

Merged
merged 3 commits into from
Sep 7, 2017

Conversation

jkakavas
Copy link
Member

If configured, map AL internal unique id as an attribute to be used
by satosa, microservices and other plugins.

If configured, map AL internal unique id as an attribute to be used
by satosa, microservices and other plugins.
@bajnokk
Copy link
Contributor

bajnokk commented Aug 16, 2017

One line was missing from the commit of @jkakavas. As this is just a trivial oversight, I prefer not to open a merge request to his tree but paste the solution here. Feel free to apply it or wait for him to update this pull request.

index ad4202e..b437129 100644
--- a/src/satosa/micro_services/account_linking.py
+++ b/src/satosa/micro_services/account_linking.py
@@ -32,6 +32,7 @@ class AccountLinking(ResponseMicroService):
         self.redirect_url = config["redirect_url"]
         self.signing_key = RSAKey(key=rsa_load(config["sign_key"]), use="sig", alg="RS256")
         self.endpoint = "/handle_account_linking"
+        self.config = config
         logger.info("Account linking is active")

     def _handle_al_response(self, context):```

@@ -52,6 +53,10 @@ def _handle_al_response(self, context):
satosa_logging(logger, logging.INFO, "issuer/id pair is linked in AL service",
context.state)
internal_response.user_id = message
id_to_attr = self.config.get("id_to_attr", None)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

config is static and set at initialisation time through __init__. I propose we move this to __init__ too, as id_to_attr is not going to change and calling .get over and over is only wasting cpu cycles.

@@ -32,6 +32,7 @@ def __init__(self, config, *args, **kwargs):
          self.redirect_url = config["redirect_url"]
          self.signing_key = RSAKey(key=rsa_load(config["sign_key"]), use="sig", alg="RS256")
          self.endpoint = "/handle_account_linking"
+         self.config = config
+         self.id_to_attr = self.config.get("id_to_attr", None)
          logger.info("Account linking is active")
  
      def _handle_al_response(self, context):

then all you need to do is

if self.id_to_attr:
    internal_response.attributes[self.id_to_attr] = [message]

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm only using self.config = config in order to get id_to_attr later on, so we even could do something like

self.id_to_attr = config.get("id_to_attr", None)

@@ -52,6 +53,10 @@ def _handle_al_response(self, context):
satosa_logging(logger, logging.INFO, "issuer/id pair is linked in AL service",
context.state)
internal_response.user_id = message
id_to_attr = self.config.get("id_to_attr", None)
if id_to_attr:
internal_response.attributes[id_to_attr] = message
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Which brings me to the next point, which is that attributes are stored as an array of attribute-values. I think this should be

internal_response.attributes[id_to_attr] = [message]
#                                          ^ notice the brackets

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

good catch, thanks

@c00kiemon5ter
Copy link
Member

LGTM

@johanlundberg johanlundberg merged commit cbb6d4a into IdentityPython:master Sep 7, 2017
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants