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

extract auth to plugins, starting with ldap #3001

Open
wants to merge 1 commit into
base: master
Choose a base branch
from
Open

Conversation

grosser
Copy link
Contributor

@grosser grosser commented Oct 15, 2018

not everyone should have to include all the various gems for auth,
so let's move them into plugins and also make it possible to write a new auth method as plugin

WIP feedback welcome, still need to move tests around, but basics seem to work / code did not get too ugly

@samson/bre

@duduribeiro @jgoerz I need someone to test this so I don't break stuff ... I don't have ldap ... ideally comment on how to get a test setup going ...

Risks

  • Medium: LDAP not working

screen shot 2018-10-14 at 9 26 00 pm

not everyone should have to include all the various gems for auth,
so let's move them into plugins and also make it possible to write a new auth method as plugin
@grosser grosser requested a review from a team as a code owner October 15, 2018 04:29
get '/auth/gitlab/callback', to: 'sessions#gitlab'
get '/auth/bitbucket/callback', to: 'sessions#bitbucket'
get '/auth/:type/callback', to: 'sessions#omniauth_callback'
post '/auth/:type/callback', to: 'sessions#omniauth_callback'
Copy link
Contributor Author

Choose a reason for hiding this comment

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

any did not work and using match with a placeholder did not work either 🤷‍♂️


<% providers.each do |provider| %>
<%= link_to omniauth_path(provider.downcase.to_sym), class: "action #{provider.downcase}" do %>
<%= image_tag image_url("auth/#{provider.downcase}.png") %> Login with <%= provider %>
Copy link
Contributor Author

Choose a reason for hiding this comment

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

could also be a plugin view, but this way the UI stays consistent ...

end

def bitbucket
def omniauth_callback
Copy link
Contributor Author

Choose a reason for hiding this comment

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

github is 1-off so keeping it separate for now, but eventually it should go through here too

@duduribeiro
Copy link
Contributor

Hey @grosser 👋

I tested it and it is returning No route matches [GET] "/auth/ldap"

And I don't know why 😄 .. I need to investigate it better.

To easily setup LDAP locally, you can use a docker image from OpenLDAP.

make these changes:

diff --git a/docker-compose.yml b/docker-compose.yml
index a1cbd5708..d6ad08390 100644
--- a/docker-compose.yml
+++ b/docker-compose.yml
@@ -1,12 +1,34 @@
 version: "2"
 services:
   samson:
-    image: zendesk/samson:latest
+    build: .
     ports:
       - "3000:9080"
     volumes:
       - .:/app/
     environment:
       DATABASE_URL: "sqlite3:///app/db/development.sqlite3"
+      LDAP_HOST: ldap
       RAILS_LOG_TO_STDOUT: 1
+    links:
+      - ldap
     command: ["./script/docker_dev_server"]
+
+  ldap:
+    image: osixia/openldap
+    ports:
+      - "389"
+      - "636"
+    volumes:
+      - ./tmp/ldap_data:/var/lib/ldap
+      - ./tmp/slapd_data:/etc/ldap/slapd.d
+
+  phpldapadmin:
+    image: osixia/phpldapadmin
+    links:
+      - ldap
+    ports:
+      - "8080:80"
+      - "8043:443"
+    environment:
+      PHPLDAPADMIN_LDAP_HOSTS: ldap

and do a

docker-compose up

you can manage your LDAP at: https://localhost:8043

using these login info:

Login DN: cn=admin,dc=example,dc=org
Password: admin

After authenticating, you should create a Generic: Posix Group and after a Generic: User Account.

I will try to investigate it more. If you have more doubts, please ping me.

@grosser
Copy link
Contributor Author

grosser commented Oct 16, 2018 via email

@grosser
Copy link
Contributor Author

grosser commented Oct 21, 2018

the /auth/ldap comes from calling provider OmniAuth::Strategies::LDAP so I must have gotten the order wrong somehow :)

1 similar comment
@grosser
Copy link
Contributor Author

grosser commented Oct 21, 2018

the /auth/ldap comes from calling provider OmniAuth::Strategies::LDAP so I must have gotten the order wrong somehow :)

@Lazyshot Lazyshot requested review from a team and removed request for a team November 23, 2021 14:09
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.

None yet

2 participants