Issue #242 resolved

PermissionUtil and branches have bugs

Ivan Lam avatarIvan Lam created an issue
  1. boolean isOwner = PermissionUtil.hasPermission(event.getRepository(), securityContextProvider, PermissionType.OWNER) if the user is admin and is not owner, it return true.
  2. changeset.getBranches() If user create a new branch at local and push to server. Server will get the branch name as master not the branch real name.

Comments (28)

  1. Ivan Lam

    Yes. 1. create a new branch at local. The branch is not exist at remote. Switch to the new branch. 2. push the branch to remote server. 3. at remote server, using "changeset.getBranches()". And get the branch name is master, not the real branch name.

  2. Sebastian Sdorra

    Ok, i've done some tests. But i could not reproduce this issue. There are duplicated changesets, but there is no wrong branch. I've tested with the following hook:

    @Extension
    public class GitHookTest extends PostReceiveRepositoryHook
    {
      @Override
      public void onEvent(RepositoryHookEvent event)
      {
        System.out.println("Changesets: ");
        for ( Changeset c : event.getChangesets() )
        {
          System.out.println( "---" );
          System.out.append("id: ").println( c.getId() );
          System.out.append("desc: ").println( c.getDescription() );
          System.out.append("branches: ").println( c.getBranches() );
        }
      }
    }
    

    I've create the repository with the following commands:

    $ git clone http://localhost:8080/scm/git/test
    Cloning into 'test'...
    Username for 'http://localhost:8080': scmadmin
    Password for 'http://scmadmin@localhost:8080': 
    warning: You appear to have cloned an empty repository.
    $ cd test 
    $ echo a > a.txt
    $ git add a.txt 
    $ git commit -m 'added a'
    [master (root-commit) 3612f95] added a
     1 file changed, 1 insertion(+)
     create mode 100644 a.txt
    $ git branch b
    $ git checkout b
    Switched to branch 'b'
    $ echo b > b.txt
    $ git add b.txt 
    $ git commit -m 'added b'
    [b ec7118b] added b
     1 file changed, 1 insertion(+)
     create mode 100644 b.txt
    $ git checkout master
    Switched to branch 'master'
    $ git branch c
    $ git checkout c
    Switched to branch 'c'
    $ echo c > c.txt
    $ git add c.txt 
    $ git commit -m 'added c'
    [c 91f940b] added c
     1 file changed, 1 insertion(+)
     create mode 100644 c.txt
    $ git checkout master 
    Switched to branch 'master'
    $ git push origin --all
    Username for 'http://localhost:8080': scmadmin
    Password for 'http://scmadmin@localhost:8080': 
    Counting objects: 9, done.
    Delta compression using up to 4 threads.
    Compressing objects: 100% (5/5), done.
    Writing objects: 100% (9/9), 612 bytes, done.
    Total 9 (delta 1), reused 0 (delta 0)
    remote: Resolving deltas: 100% (1/1)
    remote: Updating references: 100% (3/3)
    To http://localhost:8080/scm/git/test
     * [new branch]      b -> b
     * [new branch]      c -> c
     * [new branch]      master -> master
    

    And here is the output of the hook:

    Changesets: 
    ---
    id: 3612f95778ba28d7b9de
    desc: added a
    branches: [master]
    ---
    id: 91f940beb9f765848374
    desc: added c
    branches: [c]
    ---
    id: 3612f95778ba28d7b9de
    desc: added a
    branches: [master]
    ---
    id: ec7118b58d748ef18df6
    desc: added b
    branches: [b]
    ---
    id: 3612f95778ba28d7b9de
    desc: added a
    branches: [master]
    

    The branches are correct for each changeset.

  3. Ivan Lam

    Please try this way. The error is throw by me when I get the branch name is master. I use PreReceiveRepositoryHook instead of PostReceiveRepositoryHook, is it OK?

    [root@server2 test]# git branch branch_2
    [root@server2 test]# git checkout branch_2
    Switched to branch 'branch_2'
    [root@server2 test]# git push origin branch_2
    Total 0 (delta 0), reused 0 (delta 0)
    remote: error: [SCM] Only owner can post master changesets.
    
  4. Sebastian Sdorra

    Yes, a PreReceiveRepositoryHook is also ok.

    The problem is that git resends the changeset where the branch was created for every new branch.

    You could see this very good in my last post. The changeset 3612f95778ba28d7b9de (added a) was resend for every new branch.

  5. Ivan Lam

    I don't want to change master's file. I just create a new branch(branch_2) and just push it to remote by using follow command.

    git push origin branch_2
    

    I hope the server can just get the real branch name. Because I just let owner to deal with master. Other people can just deal with other branches. What can I do?

  6. Sebastian Sdorra

    You get the real name of the branches, but the client resends the changeset from which the branch was created. This means that you modify the master branch, if you create a branch from the master branch. You could push to an existing branch without permissions for the master branch. But you could not create a branch from the master branch, without write permissions for the master branch. I will try to block the modify branch changesets, but i'm not sure if this is possible.

  7. Ivan Lam

    I find a very important problem.

    [root@server2 test]# git branch 02
    [root@server2 test]# git checkout 02
    Switched to branch '02'
    [root@server2 test]# git push origin 03
    [root@server2 test]# git branch 03
    [root@server2 test]# git checkout 03
    Switched to branch '03'
    

    And then I modify a file and commit it. I push it again.

    [root@server2 test]# git push origin 03
    

    When I push 03, I print the branches and author's info. It shows like this.

    10:52:08.007 [catalina-exec-46] ERROR  - author:ivan.lam
    10:52:08.007 [catalina-exec-46] ERROR  - author:root
    10:52:08.007 [catalina-exec-46] ERROR  - branch:02
    10:52:08.007 [catalina-exec-46] ERROR  - author:ivan.lam
    10:52:08.007 [catalina-exec-46] ERROR  - author:root
    

    You will find that the server can not get the new branch info. Just get the old info.

  8. Ivan Lam

    I have tested it. And that version is fine. But I have no idea why it's change set have so many info. The branch is just create and push.

    09:45:04.198 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.198 [catalina-exec-7] ERROR  - branch:04
    09:45:04.199 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.199 [catalina-exec-7] ERROR  - branch:04
    09:45:04.199 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.199 [catalina-exec-7] ERROR  - branch:04
    09:45:04.199 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.199 [catalina-exec-7] ERROR  - branch:04
    09:45:04.199 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.200 [catalina-exec-7] ERROR  - branch:04
    09:45:04.200 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.200 [catalina-exec-7] ERROR  - branch:04
    09:45:04.200 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.200 [catalina-exec-7] ERROR  - branch:04
    09:45:04.200 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.200 [catalina-exec-7] ERROR  - branch:04
    09:45:04.200 [catalina-exec-7] ERROR  - author:ivan.lam
    09:45:04.200 [catalina-exec-7] ERROR  - branch:04
    09:45:04.200 [catalina-exec-7] ERROR  - author:root
    
  9. Ivan Lam

    It have 2 problems.

    1, I find this branch is merge to master. When I update to 1.20 , it can not get any branch infos.

        for (Changeset changeset : event.getChangesets()) {
        for (String branch : changeset.getBranches()) {
    

    2, I used your package. http://download.scm-manager.org/issues/242/scm-webapp-2012092002.war . When I throw a ScmSecurityException, it will not stop push action.

        for (Changeset changeset : event.getChangesets()) {
        for (String branch : changeset.getBranches()) {
            logger.debug("branch:" + branch);
            if (branch.equals("master")) {
            throw new ScmSecurityException(
                "Only owner can post master changesets.");
            }
        }
        }
    
  10. Ivan Lam

    package com.iwode.ownerauth;

    import org.slf4j.Logger; import org.slf4j.LoggerFactory;

    import sonia.scm.plugin.ext.Extension; import sonia.scm.repository.Changeset; import sonia.scm.repository.PermissionType; import sonia.scm.repository.PermissionUtil; import sonia.scm.repository.PreReceiveRepositoryHook; import sonia.scm.repository.Repository; import sonia.scm.repository.RepositoryHookEvent; import sonia.scm.security.ScmSecurityException; import sonia.scm.web.security.WebSecurityContext;

    import com.google.inject.Inject; import com.google.inject.Provider;

    extension public class OwnerAuthHook extends PreReceiveRepositoryHook {

    private static final Logger logger = LoggerFactory
        .getLogger(OwnerAuthHook.class);
    
    /** Field description */
    private Provider<WebSecurityContext> securityContextProvider;
    
    /**
     * Constructs ...
     * 
     * 
     * @param securityContextProvider
     */
    @Inject
    public OwnerAuthHook(Provider<WebSecurityContext> securityContextProvider) {
    this.securityContextProvider = securityContextProvider;
    logger.debug("init");
    }
    
    /**
     * Method description
     * 
     * 
     * @param event
     */
    @Override
    public void onEvent(RepositoryHookEvent event) {
    boolean isEnabled = this.isEnabled(event.getRepository());
    boolean isOwner = PermissionUtil.hasPermission(event.getRepository(),
        securityContextProvider, PermissionType.OWNER);
    logger.debug("enable:" + isEnabled);
    logger.debug("owner:" + isOwner);
    if (!isOwner) {
        for (Changeset changeset : event.getChangesets()) {
        for (String branch : changeset.getBranches()) {
            logger.debug("branch:" + branch);
            if (branch.equals("master")) {
            throw new ScmSecurityException(
                "Only owner can post master changesets.");
            }
        }
        }
    }
    }
    
    private boolean isEnabled(Repository repository) {
    return Boolean.valueOf(repository.getProperty("ownerauthEnabled"));
    }
    

    }

  11. Log in to comment
Tip: Filter by directory path e.g. /media app.js to search for public/media/app.js.
Tip: Use camelCasing e.g. ProjME to search for ProjectModifiedEvent.java.
Tip: Filter by extension type e.g. /repo .js to search for all .js files in the /repo directory.
Tip: Separate your search with spaces e.g. /ssh pom.xml to search for src/ssh/pom.xml.
Tip: Use ↑ and ↓ arrow keys to navigate and return to view the file.
Tip: You can also navigate files with Ctrl+j (next) and Ctrl+k (previous) and view the file with Ctrl+o.
Tip: You can also navigate files with Alt+j (next) and Alt+k (previous) and view the file with Alt+o.