jech / galene

The Galène videoconference server
https://galene.org
MIT License
944 stars 130 forks source link

LDAP authentication #85

Closed georgesk closed 3 years ago

georgesk commented 3 years ago

Here is an attempt to add LDAP authentication for Galène. Main features are:

Here is an example of a working descrition (file groups/demo.json). There are no "manually created" accounts:

{
    "description" : "Salon de démonstration numéro 1",
    "autolock": true,
    "ldapurl": "ldap://example.edu:1389",
    "ldapbase": "dc=lycee,dc=jb",
    "ldapuserbranch": "cn=Users,dc=lycee,dc=jb",
    "ldapbinduser": "personne",
    "ldapbindpassword": "donottellsecret",
    "op_ldap": [{"field": "memberOf", "begins": "CN=profs"}],
    "presenter_ldap": [{"field": "memberOf", "begins": "CN=profs"}]
}
jech commented 3 years ago

Could you please squash and rebase, so I can review your patch?

georgesk commented 3 years ago

Hello, I had a look at https://blog.carbonfive.com/always-squash-and-rebase-your-git-commits/; however, I fetched https://github.com/jech/galene.git as upstream/master yesterday and merged it in my file tree, and my branch seems to have no conflict with yours.

When I take a look at https://github.com/jech/galene/pull/85/files, I wonder which other detail would be available for review the changes, if I practice a "squash & rebase"?

Here is a summary of the changes:

  1. introduced a Makefile (simple utility for me, discard it)
  2. added Givenpassword() string to the Challengeable interface: sorry, I tried hard to make modifications inside the function Challenge defined in the file rtpconn/webclient.go, but I found no mean to access the password provided by the user in the welcome page, in the context of this function...
  3. the biggest part of the patch touches the file group/group.go: I extended the properties of Description to allow one to record useful information about a LDAP directory in JSON group description files. One can also provide basic means to decide who is OP, presenter, and so on, by simple patterns. Further on, I wrote some code to attempt LDAP authentication before file authentication, if the property ldapurl is mentioned in the group's description.
  4. additionally, I had to implement the function Givenpassword() in other files which use the Challengeable interface
jech commented 3 years ago

Please be so kind as to rebase and squash your changes. The simplest way is

git rebase master
git reset master
git commit -a

Also, please remove the Makefile (you can do that before you commit).

georgesk commented 3 years ago

Done:

georgesk@georges:~/developpement/galene/galene$ git status
Sur la branche master
Votre branche est à jour avec 'origin/master'.

rien à valider, la copie de travail est propre
georgesk@georges:~/developpement/galene/galene$ git rebase master
La branche courante master est à jour.
georgesk@georges:~/developpement/galene/galene$ git reset master
georgesk@georges:~/developpement/galene/galene$ git status
Sur la branche master
Votre branche est à jour avec 'origin/master'.

rien à valider, la copie de travail est propre

Did I make white magics? or invisible magics? I shall try to learn more about Git. However, git log gives me no indication about a significant change. I removed the file Makefile as requested, and committed the changes.

jech commented 3 years ago

La branche courante master est à jour.

Il faut faire ça sur la branche de développement, pas la branche master.

georgesk commented 3 years ago

Bonjour,

Juliusz Chroboczek a écrit :

La branche courante master est à jour.

Il faut faire ça sur la branche de développement, pas la branche master.

Nous n'avons probablement pas les mêmes pratiques. Je n'ai pas vraiment séparé une branche de développement de mon côté ; j'ai fourché le dépôt jech/galene dans un dépôt georgesk/galene, puis cloné ça chez moi.

Tous les changements chez moi ont été faits dans la branche master.

Comme le dépôt jech/galene avait quelques nouveaux commits, je les ai récupérés à moment donné : fetch upstream, puis merge upstream/main comme expliqué à https://docs.github.com/en/github/collaborating-with-issues-and-pull-requests/syncing-a-fork

Voilà ce que dit git branch chez moi :

***@***.***:~/developpement/galene/galene$ git branch
  lockify
* master

(oublions la branche lockify, elle n'a rien à voir avec l'authentification ldap, c'est un brouillon pour une autre bidouille personnelle)

et voilà la configuration que j'utilise :

***@***.***:~/developpement/galene/galene$ cat .git/config
[core]
    repositoryformatversion = 0
    filemode = true
    bare = false
    logallrefupdates = true
[remote "origin"]
    url = ***@***.***:georgesk/galene.git
    fetch = +refs/heads/*:refs/remotes/origin/*
[branch "master"]
    remote = origin
    merge = refs/heads/master
[remote "upstream"]
    url = https://github.com/jech/galene.git
    fetch = +refs/heads/*:refs/remotes/upstream/*

Autrement dit, je comprends mal dans ce contexte, comment je pourrais « faire ça sur la branche de développement, pas la branche master »

Amitiés, Georges.

jech commented 3 years ago
git branch ldap-old
git reset upstream/master
git commit -a

If something goes wrong, you can then come back to your original state by doing

git reset --hard ldap-old
georgesk commented 3 years ago

Hello, I did:

git branch ldap-old
git reset upstream/master
git commit -am "Merge branch 'master' of github.com:georgesk/galene"

However at that point it was impossible to push changes. I had to pull from my repository again (which introduced no diff!) Then I could push it all.

As far as I can tell, the diffs attached to this pull request did not change.

jech commented 3 years ago

Closing for now, please reopen if you decide to work on this again.

georgesk commented 3 years ago

Hello Juliusz,

I opened this new pull request: " LDAP authentication, again #104"

I have not understood what went wrong with the previous try, and hope that this request is acceptable.

Best regards, Georges.

Juliusz Chroboczek a écrit :

Closing for now, please reopen if you decide to work on this again.

-- You are receiving this because you authored the thread. Reply to this email directly or view it on GitHub: https://github.com/jech/galene/pull/85#issuecomment-907612638

-- Georges KHAZNADAR et Jocelyne FOURNIER 22 rue des mouettes, 59240 Dunkerque France. Téléphone +33 (0)3 28 29 17 70