Skip to content
This repository has been archived by the owner on Dec 4, 2018. It is now read-only.

prevent crowbar batch from locking itself and the user out #1336

Closed
wants to merge 2 commits into from

Conversation

aspiers
Copy link
Member

@aspiers aspiers commented Jul 30, 2015

If crowbar batch is used to export the crowbar proposal from one cloud and apply it to another, it will change the machine-install user's password, which is typically the one used to run crowbar batch. This would break any future HTTP digest authentications against the Crowbar REST API, including ones invoked immediately after the change, by the still running crowbar batch process. So if the user inadvertently attempts to change the password they're currently relying on, simply output a warning and ignore the attempt.

Also improve error reporting when retrieving aliases.

@aspiers
Copy link
Member Author

aspiers commented Jul 30, 2015

This is not right yet.

@aspiers aspiers changed the title prevent crowbar batch from locking itself and the user out [WIP] prevent crowbar batch from locking itself and the user out Jul 30, 2015
@aspiers aspiers changed the title [WIP] prevent crowbar batch from locking itself and the user out prevent crowbar batch from locking itself and the user out Jul 30, 2015
@aspiers
Copy link
Member Author

aspiers commented Jul 30, 2015

OK this should be right now - needs a quick test though.

#"from #{@password} to #{users[@username]['password']} " + \
"which would lock myself out!"
end
to_merge['attributes'][barclamp]['users'].delete @username
Copy link
Contributor

Choose a reason for hiding this comment

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

While testing this I got a

/opt/dell/bin/barclamp_lib.rb:536:in `eval': undefined local variable or method `to_merge' for main:Object (NameError)

Changing it to an instance variable @to_merge (in all its lines ofc) fixes this.

Second issue in this line: barclamp is unknown as-well. Making it a method argument fixed it for me.

Copy link
Member Author

Choose a reason for hiding this comment

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

Thanks for the catch but there is a better fix for than these ;-)

Adam Spiers added 2 commits August 4, 2015 16:19
If crowbar batch is used to export the crowbar proposal from one cloud
and apply it to another, it will change the machine-install user's
password, which is typically the one used to run crowbar batch.  This
would break any future HTTP digest authentications against the Crowbar
REST API, including ones invoked immediately after the change, by the
still running crowbar batch process.  So if the user inadvertently
attempts to change the password they're currently relying on, simply
output a warning and ignore the attempt.
e.g. if the username/password is wrong, it needs to be clear to the user
that authentication failed.
@thutterer
Copy link
Contributor

Tested again. Works.
+1

@rsalevsky
Copy link
Member

LGTM but I have not much experience with crowbar_batch

@rsalevsky
Copy link
Member

@aspiers, this repository got merged into crowbar-core.
Please open a new pull request there. Do NOT merge here anymore!

@aspiers
Copy link
Member Author

aspiers commented Sep 14, 2015

Superseded by crowbar/crowbar-core#35

@aspiers aspiers closed this Sep 14, 2015
@aspiers aspiers deleted the improve-batch-auth branch September 16, 2015 10:08
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
3 participants