raphendyr / gitlabhq

This fork contained implementation for PAM based login to Gitlab. It's left as archived as the feature was never merged.
MIT License
1 stars 1 forks source link

repository access error using PAM #6

Closed b0ric closed 10 years ago

b0ric commented 10 years ago

Hi raphendyr! Thanks for you patch to gitlab so that everybody is able to use PAM auth with gitlab! And I found one issue with recent 6-3-pam branch. When I work with git (push/pull/etc..), gitlab-shell issues api requests to check whether this user can access repository or not. Smth. like: /api/v3/internal/allowed?key_id=22&action=git-receive-pack&ref=_any&project=proj/proj Handler of this request will call Gitlab::PAM::User.blocked?(user.extern_uid) to check whether user is blocked or not:

          begin
            Etc.getpwnam(uid)
          rescue ArgumentError => ex
            return false
          end

          return true

and, I guess it should be vice versa:

          begin
            Etc.getpwnam(uid)
          rescue ArgumentError => ex
            return true
          end

          return false

and another catch is that PAM info is not always available. E.g. I have one server where PAM auth is enabled through Kerberos plugin: libpam-krb5. So in this case there will be no info available in /etc/passwd file. So my guess is that it's better simply to remove begin/rescue block. What do you think?

raphendyr commented 10 years ago

I reversed the logic in 6-4-pam branch. Though I don't know why does this work or doesn't in our system. Might be that there is bug in some other place. Need to debug.

Does your machine provide correct information with command getent passwd <username>. Our /etc/passwd dosn't contain that information, but we have nss_cache or something like that.

Apparently Etc.getpwnam(uid) works in our environment (used by omniauth-pam).

raphendyr commented 10 years ago

Also. Thank you for submitting these issues. I'm sorry hat I haven't noticed them earlier...

b0ric commented 10 years ago

Yep, I see, 6-4-pam is correct now, thanx! getent passwd <username> do not provide any information on my machine. nss_cache seems like a good solution but I don't have LDAP set up - only Kerberos accounts so far.

Actually I run Gitlab on machine where all this information exists and Etc.getpwnam(uid) works correctly there. I just bumped into this issue while developing another web application that is on another server. That's why I thought it was worth reporting.

b0ric commented 10 years ago

Also, I guess it's safe to close this issue, right?

raphendyr commented 10 years ago

Yep. Thanks for the clarifications. I try to keep that (testing with getent passwd) in mind if/when I ever write documentation how to use pam.