Skip to content
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

Jetty combined ldap #98

Merged
merged 1 commit into from
Sep 11, 2015
Merged

Jetty combined ldap #98

merged 1 commit into from
Sep 11, 2015

Conversation

rooty0
Copy link
Contributor

@rooty0 rooty0 commented Jun 1, 2015

Before applying this, please see #94 and #96 as these fixes were merged to this feature branch (JettyCombinedLdap).


Since Rundeck 2.5 there is a new very useful feature: LDAP "shared authentication credentials".
see http://rundeck.org/docs/administration/authenticating-users.html#ldap for more details.

This pull request provides puppet's implementation of that feature

to activate this feature $auth_types should be assigned as

  • ldap_shared

or

  • active_directory_shared

That means that rundeck will authenticate user with LDAP (or AD) and will perform authorization with FileLoginModule (realm file) to assign provided roles

@liamjbennett liamjbennett added the enhancement New feature or request label Jun 5, 2015
@rooty0
Copy link
Contributor Author

rooty0 commented Jun 9, 2015

hey @liamjbennett
Just wondering.
Do you need a help on this? Do you want me to fix something?
You need to revert only this change: https://github.com/rooty0/puppet-rundeck/commit/e2a4f4fe969029980e7be5ab6dc9c80281728d06

These changes are tested and they are works without any issues.

I using this in production at my work.

@jyaworski
Copy link
Member

Hello:

Not an owner of the repo, but can you resolve the conflicts in this PR? This also serves as a reminder to the owners.

@rooty0
Copy link
Contributor Author

rooty0 commented Sep 10, 2015

@liamjbennett c'mon, let's merge it :)
it is really useful feature, I use this module including these changes in production since June.

#94 was removed from this branch

@nibalizer
Copy link
Member

@rooty0 as @jyaworski commented, can you squash please?

@nibalizer
Copy link
Member

Looking at the change this looks fine. Happy to merge when it gets squashed.

@rooty0
Copy link
Contributor Author

rooty0 commented Sep 11, 2015

I'll try, didn't do squash before :)

@rooty0
Copy link
Contributor Author

rooty0 commented Sep 11, 2015

@nibalizer, I think I did it. Do you want me to change commit message?

@nibalizer
Copy link
Member

@rooty0 yes please can you rewrite the commit message to be one description of the complete change. Sorry to nitpick on procedure here.

@rooty0
Copy link
Contributor Author

rooty0 commented Sep 11, 2015

@nibalizer it's ok, commit message fixed. Let me know if I need to fix something else. Thanks

nibalizer added a commit that referenced this pull request Sep 11, 2015
@nibalizer nibalizer merged commit 8e59a5b into voxpupuli:master Sep 11, 2015
@rooty0 rooty0 deleted the JettyCombinedLdap branch September 11, 2015 18:53
}
if 'active_directory_shared' in $auth_types {
$_deploy_realm = true
}
Copy link
Contributor

Choose a reason for hiding this comment

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

do all of these conditions have to be met for it to become true?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

@igalic sorry, I'm afraid I don't understand your question. We need to deploy realm file when "file" or/and "ldap_shared" or/and "active_directory_shared" in use . In case of "shared" approach, we still use realm file for authorization procedure.

Copy link
Contributor

Choose a reason for hiding this comment

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

that's exactly what i was asking, @rooty0.

i was just trying to think of a more puppety way of structuring that logic.

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

Successfully merging this pull request may close these issues.

5 participants