search

Shuffle / python-apps

Apps to be used for Shuffle automation. Most of Shuffle's apps (2500+) are generated from APIs, and available in the search engine below:
https://shuffler.io/search
MIT License
100 stars 107 forks source link

Active Directory Python App Fails To Connect #330

Open ChestoOfGlen opened 1 year ago

ChestoOfGlen commented 1 year ago

Describe the bug The Active Directory Python app fails to connect with an error "automatic bind failed, invalid credentials". image

To Reproduce Steps to reproduce the behavior:

  1. Try to run the Active Directory app

Expected behavior That the app should authenticate via LDAP and successfully execute.

We have confirmed that the credentials are correct via using ldapsearch from the Orborus server. We have also pulled out the specific bit of the Python code that makes the LDAP connection and run that as a standalone piece of Python from within an instance of the app container on the Orborus server and it also successfully binds. image

Logs for the app container itself from one of our test runs. image

Logs from the shuffle-worker for the same execution. image

It looks to me like the core issue is the "TypeError wf variables: 'NoneType' object is not iterable" from the app container logs, but where exactly that error is coming from I'm not quite sure.

Almost certainly unrelated, I also note that the AD app reports a v1.0.0 and v1.0.1 (and you can select v1.0.1 as the app version in a workflow) but there is only a v1.0.0 in the repo: https://github.com/Shuffle/python-apps/tree/master/active-directory/1.0.0

ChestoOfGlen commented 1 year ago

Also figured this one out. We had a $ in the password, which once I went away and came back 5 minutes later it was suddenly obvious that Shuffle was trying to interpret it as a variable instead of a raw string. I even use that same functionality in another workflow to dynamically provide an API key. I just completely forgot about that.

I'm not sure if there is a way to be able to escape the $ in a password, or whether the easier and better solution is to just make it abundantly clear in the documentation that $ should be avoided in variable values (EG: passwords) because it will be interpreted as a variable name (unless you are using it for that purpose).

Not sure if it's really classifiable as a bug or not. Probably just a documentation update to make it clear to avoid using a $ unless you are referencing a variable. I also don't think password fields should be excluded from the variable parsing as that then removes the ability to dynamically assign a variable (EG: password, API key), which is nice to have.

felipee07 commented 1 year ago

Hi,

Use backslash to escape the $

\$password

frikky commented 1 year ago

Describe the bug The Active Directory Python app fails to connect with an error "automatic bind failed, invalid credentials". image

To Reproduce Steps to reproduce the behavior:

  1. Try to run the Active Directory app

Expected behavior That the app should authenticate via LDAP and successfully execute.

We have confirmed that the credentials are correct via using ldapsearch from the Orborus server. We have also pulled out the specific bit of the Python code that makes the LDAP connection and run that as a standalone piece of Python from within an instance of the app container on the Orborus server and it also successfully binds. image

Logs for the app container itself from one of our test runs. image

Logs from the shuffle-worker for the same execution. image

It looks to me like the core issue is the "TypeError wf variables: 'NoneType' object is not iterable" from the app container logs, but where exactly that error is coming from I'm not quite sure.

Almost certainly unrelated, I also note that the AD app reports a v1.0.0 and v1.0.1 (and you can select v1.0.1 as the app version in a workflow) but there is only a v1.0.0 in the repo: https://github.com/Shuffle/python-apps/tree/master/active-directory/1.0.0

Heyo!

Finally caught up after the holidays, ready to fix some stuff again :) I'm transferring this to the app repo and assigning it, as we indeed do need to test this properly to continue. Indeed I think you're right that it's not necessarily a bad credentials issue, and I do know this app is running in production multiple places.

@DavidtheGoliath could you test the app in one of our test environments?

ChestoOfGlen commented 1 year ago

Thanks @frikky Did you get a chance to read my reply on this issue? Figured out a workaround myself not long after submitting the issue. Not sure if there's anything specific to test in this case now I've figured out what my problem was and it isn't a bug as such, just the way that Shuffle operates.

We had a $ in the password, which once I went away and came back 5 minutes later it was suddenly obvious that Shuffle was trying to interpret it as a variable instead of a raw string. I even use that same functionality in another workflow to dynamically provide an API key. I just completely forgot about that.

I'm not sure if there is a way to be able to escape the $ in a password, or whether the easier and better solution is to just make it abundantly clear in the documentation that $ should be avoided in variable values (EG: passwords) because it will be interpreted as a variable name (unless you are using it for that purpose).

Not sure if it's really classifiable as a bug or not. Probably just a documentation update to make it clear to avoid using a $ unless you are referencing a variable. I also don't think password fields should be excluded from the variable parsing as that then removes the ability to dynamically assign a variable (EG: password, API key), which is nice to have.

frikky commented 1 year ago

Thanks @frikky Did you get a chance to read my reply on this issue? Figured out a workaround myself not long after submitting the issue. Not sure if there's anything specific to test in this case now I've figured out what my problem was and it isn't a bug as such, just the way that Shuffle operates.

We had a $ in the password, which once I went away and came back 5 minutes later it was suddenly obvious that Shuffle was trying to interpret it as a variable instead of a raw string. I even use that same functionality in another workflow to dynamically provide an API key. I just completely forgot about that.

I'm not sure if there is a way to be able to escape the $ in a password, or whether the easier and better solution is to just make it abundantly clear in the documentation that $ should be avoided in variable values (EG: passwords) because it will be interpreted as a variable name (unless you are using it for that purpose).

Not sure if it's really classifiable as a bug or not. Probably just a documentation update to make it clear to avoid using a $ unless you are referencing a variable. I also don't think password fields should be excluded from the variable parsing as that then removes the ability to dynamically assign a variable (EG: password, API key), which is nice to have.

Ooh, I didnt read it well enough. The escaping was definitely the problem then. Will have to look at how to do this by default in these cases 🤔