Skip to content

Log a meaningful error if Okta connector isn't invoked correctly #416

New issue

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

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

Already on GitHub? Sign in to your account

Closed
adorton-adobe opened this issue Oct 11, 2018 · 10 comments
Closed
Assignees
Labels
Milestone

Comments

@adorton-adobe
Copy link
Collaborator

If the Okta config file is enabled and the --connector CLI param or invocation default is not specified, log an error that this option is required. Right now the error message produced by the UST mentions that the "host" key can't be found in the LDAP config - this is inaccurate and misleading.

@adorton-adobe adorton-adobe added this to the v2.4 milestone Oct 11, 2018
@adorton-adobe adorton-adobe self-assigned this Oct 11, 2018
@adorton-adobe
Copy link
Collaborator Author

@shasibhusanJena - I see that I assigned this to you, but I don't remember if we've discussed it. Do you mind handling it? Should be a pretty easy fix. Let me know if you're able to and if you have any questions.

@shasibhusanJena
Copy link
Contributor

yesterday I was trying to find the line or places those have to make changes on the code, and formatting new string/ msg for the log. we can discuss and commit changes.

@shasibhusanJena
Copy link
Contributor

shasibhusanJena commented Jan 16, 2019

I could reproduce this error on my local, as per my understanding when I comment out

ldap: "connector-ldap.yml" or # okta: "connector-okta.yml"

we are getting a common error like "value not found for key: host in LDAP configuration"

so we could have different error stmt for different error types like

for # ldap: "connector-ldap.yml"
"Do enable LDAP configuration to fetch key and values in the user-sync-config.yml file"

for # okta: "connector-okta.yml"
"Do enable Okta configuration to fetch key and values in the user-sync-config.yml file"

@adorton-adobe plz suggest me if any better way of representation of log

@shasibhusanJena
Copy link
Contributor

it look like this is the only line will get updated.

raise self.create_assertion_error("Value not found for key: %s" % key)

@adorton-adobe
Copy link
Collaborator Author

The condition to get the error is pretty specific -

  1. Comment out the LDAP config
  2. Enable the Okta config
  3. --connector command line option is not specified
  4. connector invocation option is set to "ldap" or is blank

To detect the condition in the code and report the error, I would check that the right connector config is enabled for the value of the "connector_type" option. In this case, it would still be set to LDAP (the default). So the error message would be something like "LDAP config must be enabled for the connector type 'ldap'"

@adorton-adobe
Copy link
Collaborator Author

You don't want to modify that line. It's a generic check to ensure that required keys are present in the config. You need to raise the error prior to the config processor getting there.

@shasibhusanJena
Copy link
Contributor

below are my observations

-c C:/User_sync_Integration_16-01-2019/user-sync-config.yml --process-groups --users mapped --connector ldap
getting :- Value not found for key: host in: ldap configuration

-c C:/User_sync_Integration_16-01-2019/user-sync-config.yml --process-groups --users mapped --connector okta
getting :- CRITICAL main - Okta error querying for group: Invalid token provided

-c C:/User_sync_Integration_16-01-2019/user-sync-config.yml --process-groups --users mapped --connector
app.py: error: argument --connector: expected at least one argument

@shasibhusanJena
Copy link
Contributor

raised a fresh pull req from my branch, instead of from default one. please verify it.

#442

@shasibhusanJena
Copy link
Contributor

yes I had to do an additional change, so that I don't have to run my code in a loop, kind of optimizing

@adorton-adobe
Copy link
Collaborator Author

#443 merged

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests

2 participants