Skip to content
Snippets Groups Projects
Commit f9beaee4 authored by William Turner's avatar William Turner
Browse files

Adds complete tests for auth

parent 4a58e1ba
No related branches found
No related tags found
No related merge requests found
Hide whitespace changes
Inline Side-by-side
src/metabase/api/session.clj
+ 15
− 11
View file @ f9beaee4
......@@ -33,10 +33,11 @@
:user_id (:id user))
(events/publish-event! :user-login {:user_id (:id user), :session_id <>, :first_login (not (boolean (:last_login user)))})))
(defn- ldap-fetch-or-create-user! [first-name last-name email password]
(if-let [user (or (db/select-one [User :id :last_login] :email email) ; TODO - Update the user's password
(user/create-new-ldap-auth-user! first-name last-name email password))]
{:id (create-session! user)}))
(defn- ldap-auth-fetch-or-create-user! [first-name last-name email password]
(when-let [user (or (db/select-one [User :id :last_login] :email email)
(user/create-new-ldap-auth-user! first-name last-name email password))]
(u/prog1 {:id (create-session! user)}
(user/set-password! (:id user) password))))
;;; ## API Endpoints
......@@ -51,16 +52,19 @@
password su/NonBlankString}
(throttle/check (login-throttlers :ip-address) remote-address)
(throttle/check (login-throttlers :username) username)
;; Primitive "strategy implementation", should be reworked for #3210
(or
;; First try LDAP if it's enabled
(when (ldap/ldap-configured?)
(when-let [{:keys [first-name last-name email], :as user-info} (ldap/find-user username)]
(println user-info)
(if (ldap/verify-password user-info password)
(ldap-fetch-or-create-user! first-name last-name email password)
;; Since LDAP knows about our user, fail fast here to prevent the local strategy to be tried with a potentially outdated password
(throw (ex-info "Password did not match stored password." {:status-code 400
:errors {:password "did not match stored password"}})))))
(try
(when-let [{:keys [first-name last-name email], :as user-info} (ldap/find-user username)]
(if (ldap/verify-password user-info password)
(ldap-auth-fetch-or-create-user! first-name last-name email password)
;; Since LDAP knows about our user, fail here to prevent the local strategy to be tried with an outdated password
(throw (ex-info "Password did not match stored password." {:status-code 400
:errors {:password "did not match stored password"}}))))
(catch com.unboundid.util.LDAPSDKException e
(log/error (u/format-color 'red "Unexpected LDAP error, will fallback to local authentication") (.getMessage e)))))
;; Then try local authentication
(when-let [user (db/select-one [User :id :password_salt :password :last_login], :email username, :is_active true)]
......
test/metabase/api/session_test.clj
+ 40
− 2
View file @ f9beaee4
......@@ -11,8 +11,9 @@
[metabase.public-settings :as public-settings]
[metabase.test.data :refer :all]
[metabase.test.data.users :refer :all]
[metabase.util :as u]
[metabase.test.util :refer [random-name resolve-private-vars with-temporary-setting-values], :as tu]))
[metabase.test.integrations.ldap :refer [expect-with-ldap-server]]
[metabase.test.util :refer [random-name resolve-private-vars with-temporary-setting-values], :as tu]
[metabase.util :as u]))
;; ## POST /api/session
;; Test that we can login
......@@ -249,3 +250,40 @@
admin-email "rasta@toucans.com"]
(u/prog1 (is-session? (google-auth-fetch-or-create-user! "Rasta" "Toucan" "rasta@sf-toucannery.com"))
(db/delete! User :email "rasta@sf-toucannery.com")))) ; clean up after ourselves
;;; ------------------------------------------------------------ TESTS FOR LDAP AUTH STUFF ------------------------------------------------------------
;; Test that we can login with LDAP
(expect-with-ldap-server
true
;; delete all other sessions for the bird first, otherwise test doesn't seem to work (TODO - why?)
(do (db/simple-delete! Session, :user_id (user->id :rasta))
(tu/is-uuid-string? (:id (client :post 200 "session" (user->credentials :rasta))))))
;; Test that login will fallback to local for users not in LDAP
(expect-with-ldap-server
true
;; delete all other sessions for the bird first, otherwise test doesn't seem to work (TODO - why?)
(do (db/simple-delete! Session, :user_id (user->id :crowberto))
(tu/is-uuid-string? (:id (client :post 200 "session" (user->credentials :crowberto))))))
;; Test that login will NOT fallback for users in LDAP but with an invalid password
(expect-with-ldap-server
{:errors {:password "did not match stored password"}}
(client :post 400 "session" (user->credentials :lucky))) ; NOTE: there's a different password in LDAP for Lucky
;; Test that login will fallback to local for broken LDAP settings
;; NOTE: This will ERROR out in the logs, it's normal
(expect-with-ldap-server
true
(tu/with-temporary-setting-values [ldap-base "cn=wrong,cn=com"]
;; delete all other sessions for the bird first, otherwise test doesn't seem to work (TODO - why?)
(do (db/simple-delete! Session, :user_id (user->id :rasta))
(tu/is-uuid-string? (:id (client :post 200 "session" (user->credentials :rasta)))))))
;; Test that we can login with LDAP with new user
(expect-with-ldap-server
true
(u/prog1 (tu/is-uuid-string? (:id (client :post 200 "session" {:username "sbrown20", :password "1234"})))
(db/delete! User :email "sally.brown@metabase.com"))) ; clean up
test/metabase/api/user_test.clj
+ 7
− 3
View file @ f9beaee4
......@@ -31,7 +31,8 @@
:last_login $
:first_name "Crowberto"
:email "crowberto@metabase.com"
:google_auth false})
:google_auth false
:ldap_auth false})
(match-$ (fetch-user :lucky)
{:common_name "Lucky Pigeon"
:last_name "Pigeon"
......@@ -40,7 +41,8 @@
:last_login $
:first_name "Lucky"
:email "lucky@metabase.com"
:google_auth false})
:google_auth false
:ldap_auth false})
(match-$ (fetch-user :rasta)
{:common_name "Rasta Toucan"
:last_name "Toucan"
......@@ -49,7 +51,8 @@
:last_login $
:first_name "Rasta"
:email "rasta@metabase.com"
:google_auth false})}
:google_auth false
:ldap_auth false})}
(do
;; Delete all the other random Users we've created so far
(let [user-ids (set (map user->id [:crowberto :rasta :lucky :trashbird]))]
......@@ -131,6 +134,7 @@
:is_superuser false
:is_qbnewb true
:google_auth false
:ldap_auth false
:id $})
((user->client :rasta) :get 200 "user/current"))
......
test/metabase/integrations/ldap_test.clj
+ 11
− 12
View file @ f9beaee4
......@@ -13,51 +13,50 @@
"\\2AJohn \\28Dude\\29 Doe\\5C"
(escape-value "*John (Dude) Doe\\"))
;; Make sure the basic connection stuff works, this will throw otherwise
(expect-with-ldap-server
;; Make sure the basic connection stuff works, this will throw otherwise
;; TODO: not sure if there's an "expect no throw"
nil
(.close (get-ldap-connection)))
;; Login with everything right should succeed
(expect-with-ldap-server
;; Login with everything right should succeed
true
(ldap/verify-password "cn=Directory Manager" "password"))
;; Login with wrong password should fail
(expect-with-ldap-server
;; Login with wrong password should fail
false
(ldap/verify-password "cn=Directory Manager" "wrongpassword"))
;; Login with invalid DN should fail
(expect-with-ldap-server
;; Login with invalid DN should fail
false
(ldap/verify-password "cn=Nobody,ou=people,dc=metabase,dc=com" "password"))
;; Login for regular users should also work
(expect-with-ldap-server
;; Login for regular users should also work
true
(ldap/verify-password "cn=Sally Brown,ou=people,dc=metabase,dc=com" "1234"))
;; Login for regular users should also fail for the wrong password
(expect-with-ldap-server
;; Login for regular users should also fail for the wrong password
false
(ldap/verify-password "cn=Sally Brown,ou=people,dc=metabase,dc=com" "password"))
;; Find by username should work (given the default LDAP filter and test fixtures)
(expect-with-ldap-server
;; Find by username should work (given the default LDAP filter and test fixtures)
{:dn "cn=Sally Brown,ou=people,dc=metabase,dc=com"
:first-name "Sally"
:last-name "Brown"
:email "sally.brown@example.com"
:email "sally.brown@metabase.com"
:groups []}
(ldap/find-user "sbrown20"))
;; Find by email should also work (also given our test setup)
(expect-with-ldap-server
;; Find by email should also work (also given our test setup)
{:dn "cn=Sally Brown,ou=people,dc=metabase,dc=com"
:first-name "Sally"
:last-name "Brown"
:email "sally.brown@example.com"
:email "sally.brown@metabase.com"
:groups []}
(ldap/find-user "sally.brown@example.com"))
(ldap/find-user "sally.brown@metabase.com"))
test_resources/ldap.ldif
+ 31
− 2
View file @ f9beaee4
......@@ -19,7 +19,7 @@ cn: John Smith
sn: Smith
givenName: John
uid: jsmith1
mail: John.Smith@example.com
mail: John.Smith@metabase.com
userPassword: strongpassword
dn: cn=Sally Brown,ou=people,dc=metabase,dc=com
......@@ -31,5 +31,34 @@ cn: Sally Brown
sn: Brown
givenName: Sally
uid: sbrown20
mail: sally.brown@example.com
mail: sally.brown@metabase.com
userPassword: 1234
dn: ou=birds,dc=metabase,dc=com
objectClass: top
objectClass: organizationalUnit
ou: birds
dn: cn=Rasta Toucan,ou=birds,dc=metabase,dc=com
objectClass: top
objectClass: person
objectClass: organizationalPerson
objectClass: inetOrgPerson
cn: Rasta Toucan
givenName: Rasta
sn: Toucan
uid: rasta
mail: rasta@metabase.com
userPassword: blueberries
dn: cn=Lucky Pigeon,ou=birds,dc=metabase,dc=com
objectClass: top
objectClass: person
objectClass: organizationalPerson
objectClass: inetOrgPerson
cn: Lucky Pigeon
givenName: Lucky
sn: Pigeon
uid: lucky
mail: lucky@metabase.com
userPassword: notalmonds
0% or .
You are about to add 0 people to the discussion. Proceed with caution.
Finish editing this message first!
Please register or to comment