Skip to content

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):```

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)

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
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants