Uploaded image for project: 'Jenkins'
  1. Jenkins
  2. JENKINS-27027

Log/notify REST authentication via API token

    XMLWordPrintable

Details

    Description

      Original request: SecurityListener should be notified when ApiTokenFilter approves or denies a REST authentication attempt. jglick says that the logic should be reviewed from scratch.

      TL;DR:

      • Investigate how it works
      • Send events when it does not

      Acceptance criteria:

      Not in scope:

      • SecurityListener Audit Trail patch is not in the scope. May be used as reference impl if we need to add new API

      Attachments

        Issue Links

          Activity

            jglick Jesse Glick added a comment -

            More broadly, a filter notifying you of any HTTP connection for which Jenkins.authentication can be established.

            jglick Jesse Glick added a comment - More broadly, a filter notifying you of any HTTP connection for which Jenkins.authentication can be established.
            ifernandezcalvo Ivan Fernandez Calvo added a comment - Authentication Points https://github.com/jenkinsci/jenkins/blob/d1d1ab152bfd67a2a737d7b530eb011982157f83/core/src/main/java/hudson/security/BasicAuthenticationFilter.java#L141-L149 https://github.com/jenkinsci/jenkins/blob/d1d1ab152bfd67a2a737d7b530eb011982157f83/core/src/main/java/jenkins/security/BasicHeaderApiTokenAuthenticator.java#L28-L41 Authorization Point https://github.com/jenkinsci/jenkins/blob/d1d1ab152bfd67a2a737d7b530eb011982157f83/core/src/main/java/hudson/model/User.java#L311-L313 https://github.com/jenkinsci/jenkins/blob/d1d1ab152bfd67a2a737d7b530eb011982157f83/core/src/main/java/hudson/model/User.java#L325-L326   It should trigger something like https://github.com/jenkinsci/saml-plugin/blob/master/src/main/java/org/jenkinsci/plugins/saml/SamlSecurityRealm.java#L288  but it does not.  

            Code changed in jenkins
            User: Wadeck Follonier
            Path:
            content/doc/developer/security/index.adoc
            content/doc/developer/security/resources/_auth_flow_schema.svg
            content/doc/developer/security/resources/cli_flow.png
            content/doc/developer/security/resources/web_rest_flow.png
            http://jenkins-ci.org/commit/jenkins.io/a5ded7a1738f1e7a0f521e4ed5d685e5b6bfc34d
            Log:
            JENKINS-27027 documentation for the authentication flow inside the application for default authentication possibilities

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Wadeck Follonier Path: content/doc/developer/security/index.adoc content/doc/developer/security/resources/_auth_flow_schema.svg content/doc/developer/security/resources/cli_flow.png content/doc/developer/security/resources/web_rest_flow.png http://jenkins-ci.org/commit/jenkins.io/a5ded7a1738f1e7a0f521e4ed5d685e5b6bfc34d Log: JENKINS-27027 documentation for the authentication flow inside the application for default authentication possibilities

            Code changed in jenkins
            User: R. Tyler Croy
            Path:
            content/doc/developer/security/index.adoc
            content/doc/developer/security/resources/cli_flow.svg
            content/doc/developer/security/resources/web_rest_flow.svg
            http://jenkins-ci.org/commit/jenkins.io/b953d6c376cc099c0ed9c008c781b444f639ffe6
            Log:
            Merge pull request #1192 from Wadeck/JENKINS-27027_AUTHENTICATION_FLOW

            JENKINS-27027 Documentation for Authentication flow

            Compare: https://github.com/jenkins-infra/jenkins.io/compare/5293a1df4211...b953d6c376cc

            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: R. Tyler Croy Path: content/doc/developer/security/index.adoc content/doc/developer/security/resources/cli_flow.svg content/doc/developer/security/resources/web_rest_flow.svg http://jenkins-ci.org/commit/jenkins.io/b953d6c376cc099c0ed9c008c781b444f639ffe6 Log: Merge pull request #1192 from Wadeck/ JENKINS-27027 _AUTHENTICATION_FLOW JENKINS-27027 Documentation for Authentication flow Compare: https://github.com/jenkins-infra/jenkins.io/compare/5293a1df4211...b953d6c376cc

            Code changed in jenkins
            User: Wadeck Follonier
            Path:
            core/src/main/java/hudson/Functions.java
            core/src/main/java/hudson/cli/CLICommand.java
            core/src/main/java/hudson/cli/ClientAuthenticationCache.java
            core/src/main/java/hudson/cli/LoginCommand.java
            core/src/main/java/hudson/cli/LogoutCommand.java
            core/src/main/java/hudson/model/User.java
            core/src/main/java/hudson/security/ACL.java
            core/src/main/java/hudson/security/BasicAuthenticationFilter.java
            core/src/main/java/jenkins/security/BasicHeaderApiTokenAuthenticator.java
            core/src/main/java/jenkins/security/SecurityListener.java
            test/src/test/java/hudson/security/CliAuthenticationTest.java
            test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java
            test/src/test/java/jenkins/security/SpySecurityListener.java
            http://jenkins-ci.org/commit/jenkins/b7f42b2e59b2081782d6e51da18b0c93808d98da
            Log:
            JENKINS-27027 Notify the SecurityListener on authentication (#3074)

            • JENKINS-27026 Notify the SecurityListener in case of Token based authentication success
            • due the current version of the method, the UserDetails required for the event was not accessible. In order to stay with the same API in SecurityListener, two "protected" methods were created to split the job and let the UserDetails accessible
            • - add test to ensure the SecurityListener is called for REST Token but also for regular basic auth
            • - remove the comment about the split, will be put in GitHub comment instead
            • - add check for anonymous call instead of just putting a comment
            • remove the constructor in the dummy
            • add link to PR from Daniel to simplify a call
            • - separate the before/after to save one clear and be more explicit
            • put more meaning in the assertLastEventIs method by explicitly say we will remove the last event
            • - add comment about why we do not fire the "failedToAuthenticated" in the case of an invalid token (tips: it's because it could be a valid password)
            • - also add the authenticated trigger on legacy filter as pointed by Ivan
            • - add support of event on CLI remoting authentication
            • adjust tests by moving the helper class used to spy on events
            • - as mentioned Yvan, the code had some problems with null checking, so the approach is changed in order to encapsulate all that internal mechanism
            • - add javadoc
            • open the getUserDetailsForImpersonation from the User (will let the SSHD module to retrieve UserDetails from that)
            • - remove single quote in log messages
            • - basic corrections requested by Jesse
            • - just another typo
            • - adjust the javadoc for SecurityListener events
            • - add the link to Jenkins#Anonymous
            • - add link (not using see)
            • - update comment on the isAnonymous as we (me + Oleg) do not find a best place at the moment
            • - put the new method isAnonymous in ACL instead of Functions
            • - little typo
            • add requirement about the SecurityContext authentication
            scm_issue_link SCM/JIRA link daemon added a comment - Code changed in jenkins User: Wadeck Follonier Path: core/src/main/java/hudson/Functions.java core/src/main/java/hudson/cli/CLICommand.java core/src/main/java/hudson/cli/ClientAuthenticationCache.java core/src/main/java/hudson/cli/LoginCommand.java core/src/main/java/hudson/cli/LogoutCommand.java core/src/main/java/hudson/model/User.java core/src/main/java/hudson/security/ACL.java core/src/main/java/hudson/security/BasicAuthenticationFilter.java core/src/main/java/jenkins/security/BasicHeaderApiTokenAuthenticator.java core/src/main/java/jenkins/security/SecurityListener.java test/src/test/java/hudson/security/CliAuthenticationTest.java test/src/test/java/jenkins/security/BasicHeaderProcessorTest.java test/src/test/java/jenkins/security/SpySecurityListener.java http://jenkins-ci.org/commit/jenkins/b7f42b2e59b2081782d6e51da18b0c93808d98da Log: JENKINS-27027 Notify the SecurityListener on authentication (#3074) JENKINS-27026 Notify the SecurityListener in case of Token based authentication success due the current version of the method, the UserDetails required for the event was not accessible. In order to stay with the same API in SecurityListener, two "protected" methods were created to split the job and let the UserDetails accessible - add test to ensure the SecurityListener is called for REST Token but also for regular basic auth - remove the comment about the split, will be put in GitHub comment instead - add check for anonymous call instead of just putting a comment remove the constructor in the dummy add link to PR from Daniel to simplify a call - separate the before/after to save one clear and be more explicit put more meaning in the assertLastEventIs method by explicitly say we will remove the last event - add comment about why we do not fire the "failedToAuthenticated" in the case of an invalid token (tips: it's because it could be a valid password) - also add the authenticated trigger on legacy filter as pointed by Ivan - add support of event on CLI remoting authentication adjust tests by moving the helper class used to spy on events - as mentioned Yvan, the code had some problems with null checking, so the approach is changed in order to encapsulate all that internal mechanism - add javadoc open the getUserDetailsForImpersonation from the User (will let the SSHD module to retrieve UserDetails from that) - remove single quote in log messages - basic corrections requested by Jesse - just another typo - adjust the javadoc for SecurityListener events - add the link to Jenkins#Anonymous - add link (not using see) - update comment on the isAnonymous as we (me + Oleg) do not find a best place at the moment - put the new method isAnonymous in ACL instead of Functions - little typo add requirement about the SecurityContext authentication

            People

              wfollonier Wadeck Follonier
              jglick Jesse Glick
              Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

                Created:
                Updated:
                Resolved: