Sling
  1. Sling
  2. SLING-2944

Replace administrative login by service-based login

    Details

      Description

      From the start Sling tried to solve the problem of providing services access to the repository and resource tree without having to hard code and configure any passwords. This was done first with the SlingRepository.loginAdministrative and later with the ResourceResolverFactory.getAdministrativeResourceResolver methods.

      Over time this mechanism proved to be the hammer to hit all nails. Particularly these methods while truly useful have the disadvantage of providing full administrative privileges to services where just some specific kind of privilege would be enough.

      For example for the JSP compiler it would be enough to be able to read the JSP source scripts and write the Java classes out to the JSP compiler's target location. Other access is not required. Similarly to manage users user management privileges are enough and no access to /content is really required.

      To solve this problem a new API for Service Authentication has been proposed at https://cwiki.apache.org/confluence/display/SLING/Service+Authentication. The prototype of which is implemented in http://svn.apache.org/repos/asf/sling/whiteboard/fmeschbe/deprecate_login_administrative.

      This issue is about merging the prototype code back into trunk and thus fully implementing the feature.

      1. serviceusermapper.tgz
        6 kB
        Felix Meschberger
      2. SLING-2944.patch
        77 kB
        Felix Meschberger

        Issue Links

          Activity

          Hide
          Carsten Ziegeler added a comment -

          Felix Meschberger Can we close this issue? It seems everything is implemented

          Show
          Carsten Ziegeler added a comment - Felix Meschberger Can we close this issue? It seems everything is implemented
          Hide
          Carsten Ziegeler added a comment -

          Apart from the service implementation, no java client code needs this method - that's why I removed it. So it's completely up to the service implementation how to do the mapping.
          I've changed the log messages of the clients using this code to not assume how this is done in the service, so they are just outputting the bundle and the sub service name now as separate information.

          Show
          Carsten Ziegeler added a comment - Apart from the service implementation, no java client code needs this method - that's why I removed it. So it's completely up to the service implementation how to do the mapping. I've changed the log messages of the clients using this code to not assume how this is done in the service, so they are just outputting the bundle and the sub service name now as separate information.
          Hide
          Felix Meschberger added a comment -

          IMHO this is not really utility method but it is a question of abstraction.

          The getServiceUserName method takes a bundle and an optional sub-service name and maps this a user name. This implies that the actual extraction of the required service information is abstracted in the service and there is no API documentation that the bundle symbolic name is taken for this.

          I strongly suggest to re-add the method to keep supporting the abstraction.

          Show
          Felix Meschberger added a comment - IMHO this is not really utility method but it is a question of abstraction. The getServiceUserName method takes a bundle and an optional sub-service name and maps this a user name. This implies that the actual extraction of the required service information is abstracted in the service and there is no API documentation that the bundle symbolic name is taken for this. I strongly suggest to re-add the method to keep supporting the abstraction.
          Hide
          Carsten Ziegeler added a comment -

          I think this method makes not that much sense if we don't support the header as it's basically concatenating the bundle symbolic name with the service id. So this is more an utility method than a service method.
          But if you think that it's worth having it in the service, we can add it back again

          Show
          Carsten Ziegeler added a comment - I think this method makes not that much sense if we don't support the header as it's basically concatenating the bundle symbolic name with the service id. So this is more an utility method than a service method. But if you think that it's worth having it in the service, we can add it back again
          Hide
          Felix Meschberger added a comment -

          Carsten Ziegeler Why did you remove the getServiceID method ? This is unrelated to the bundle header.

          Show
          Felix Meschberger added a comment - Carsten Ziegeler Why did you remove the getServiceID method ? This is unrelated to the bundle header.
          Hide
          Carsten Ziegeler added a comment -

          I've removed the header stuff in rev 1520522

          Show
          Carsten Ziegeler added a comment - I've removed the header stuff in rev 1520522
          Hide
          Felix Meschberger added a comment -

          Right using a service does not work: Not only is a service property unsafe, its also worthless: Its not allways a service desiring repository or resource tree access. It could also be a simple immediate component which happily sits in the background doing some cleanup (for example).

          But I am ok with removing the header stuff for now.

          Show
          Felix Meschberger added a comment - Right using a service does not work: Not only is a service property unsafe, its also worthless: Its not allways a service desiring repository or resource tree access. It could also be a simple immediate component which happily sits in the background doing some cleanup (for example). But I am ok with removing the header stuff for now.
          Hide
          Carsten Ziegeler added a comment -

          Right - I think the point is to be able to configure this without a specific packaging (bundling) of services - so we rather want to configure this based on a service than on a bundle - as the service might move from one bundle to another. Unfortunately we only get support for the client bundle through a ServiceFactory. On the other hand we can't rely alone on a service identifier (comparable to the PID) as this would be open to the same "attack" as the additional header.
          So the only thing which is quiet safe to use is the bundle symbolic name as we have it right now.
          I think it's better to be a little bit safer with the burden of potentially more configuration if sub services spread across bundles. So I would remove the header

          Show
          Carsten Ziegeler added a comment - Right - I think the point is to be able to configure this without a specific packaging (bundling) of services - so we rather want to configure this based on a service than on a bundle - as the service might move from one bundle to another. Unfortunately we only get support for the client bundle through a ServiceFactory. On the other hand we can't rely alone on a service identifier (comparable to the PID) as this would be open to the same "attack" as the additional header. So the only thing which is quiet safe to use is the bundle symbolic name as we have it right now. I think it's better to be a little bit safer with the burden of potentially more configuration if sub services spread across bundles. So I would remove the header
          Hide
          Felix Meschberger added a comment -

          Hmm, excellent point.

          The goal of the header is to allow for multiple bundles to cooperate on the same service – the MTA for example where the sub services may be spread accross bundles.

          WDYT ?

          Show
          Felix Meschberger added a comment - Hmm, excellent point. The goal of the header is to allow for multiple bundles to cooperate on the same service – the MTA for example where the sub services may be spread accross bundles. WDYT ?
          Hide
          Carsten Ziegeler added a comment -

          I'm wondering if we shouldn't remove the support for a header in the manifest and simply rely on bundle symbolic name? E.g. if a dev knows that the Sling eventing bundle uses this mechanism to get an admin session (or whatever is configured), the dev can simply use the header entry and use the symbolic name of the eventing bundle as the value, using the same configuration as the Sling eventing bundle.
          It's try that once a bundle can be deployed, more or less everything is possible, but still just relying on the symbolic name makes this concept a little bit easier, doesn't open the above mentioned door and doesn't make it's usage more difficult.

          Show
          Carsten Ziegeler added a comment - I'm wondering if we shouldn't remove the support for a header in the manifest and simply rely on bundle symbolic name? E.g. if a dev knows that the Sling eventing bundle uses this mechanism to get an admin session (or whatever is configured), the dev can simply use the header entry and use the symbolic name of the eventing bundle as the value, using the same configuration as the Sling eventing bundle. It's try that once a bundle can be deployed, more or less everything is possible, but still just relying on the symbolic name makes this concept a little bit easier, doesn't open the above mentioned door and doesn't make it's usage more difficult.
          Hide
          Felix Meschberger added a comment - - edited

          Fixed Servlet Resolver in Rev. 1510541:

          Sling API 2.4.0 is not required and probably only has been updated to make sure the import version range for the Resource API is correct. Given SLING-2993 we should actually provide proper annotation of implemented API for the bundle plugin to properly devise the import version range.

          For now removing the 'provide:=true' import tag solves this issue, since we only implement the ResourceProvider interface (intended to be @ConsumerType) and extend AbstractSlingResource (which is safe to extend in @ConsumerType fashion).

          Same for Bundle Resource Provider in Rev. 1510562
          and Filesystem Resource Provider in Rev. 1510565
          and Jackrabbit UserManager in Rev. 1510567
          and make sure Filesystem and Bundle Resource provider are used in the builder in Rev. 1510569

          Show
          Felix Meschberger added a comment - - edited Fixed Servlet Resolver in Rev. 1510541: Sling API 2.4.0 is not required and probably only has been updated to make sure the import version range for the Resource API is correct. Given SLING-2993 we should actually provide proper annotation of implemented API for the bundle plugin to properly devise the import version range. For now removing the 'provide:=true' import tag solves this issue, since we only implement the ResourceProvider interface (intended to be @ConsumerType) and extend AbstractSlingResource (which is safe to extend in @ConsumerType fashion). Same for Bundle Resource Provider in Rev. 1510562 and Filesystem Resource Provider in Rev. 1510565 and Jackrabbit UserManager in Rev. 1510567 and make sure Filesystem and Bundle Resource provider are used in the builder in Rev. 1510569
          Hide
          Felix Meschberger added a comment -

          The following bundles have also to be updated:

          • Filesystem Resource Provider
          • Bundle ResourceProvider
          • Servlet Resolver
          • Jackrabbit UserManager

          These bundles implement the ResourceProvider interface and extend the AbstractResource class and thus have got a narrow import range of [2.3,2.4).

          I think this import range is not really appropriate for these interfaces: In OSGi Versioning speak these would be "Consumer Type" types. Thus these are intended to be implemented by consumers of the API and thus should be changed with care. See SLING-2993

          Show
          Felix Meschberger added a comment - The following bundles have also to be updated: Filesystem Resource Provider Bundle ResourceProvider Servlet Resolver Jackrabbit UserManager These bundles implement the ResourceProvider interface and extend the AbstractResource class and thus have got a narrow import range of [2.3,2.4). I think this import range is not really appropriate for these interfaces: In OSGi Versioning speak these would be "Consumer Type" types. Thus these are intended to be implemented by consumers of the API and thus should be changed with care. See SLING-2993
          Hide
          Felix Meschberger added a comment -

          Reapplied the temporarily removed changes to the Jackrabbit Server bundle in Rev. 1510453

          Show
          Felix Meschberger added a comment - Reapplied the temporarily removed changes to the Jackrabbit Server bundle in Rev. 1510453
          Hide
          Felix Meschberger added a comment -

          Temporarily reverted the SlingRepository implementation in Jackrabbit Server in Rev. 1510446 to cut a 2.1.2 release first (see SLING-2923)

          Show
          Felix Meschberger added a comment - Temporarily reverted the SlingRepository implementation in Jackrabbit Server in Rev. 1510446 to cut a 2.1.2 release first (see SLING-2923 )
          Hide
          Felix Meschberger added a comment -

          Committed a first batch of implementations in Rev. 1510413:

          • Add ServiceUserMapper service and implementation bundle
          • Add service login methods to ResourceResolverFactory and SlingRepository
          • Add implementations of new methods
          Show
          Felix Meschberger added a comment - Committed a first batch of implementations in Rev. 1510413: Add ServiceUserMapper service and implementation bundle Add service login methods to ResourceResolverFactory and SlingRepository Add implementations of new methods
          Hide
          Felix Meschberger added a comment -

          Prepared initial documentation in Rev. 1510382

          Show
          Felix Meschberger added a comment - Prepared initial documentation in Rev. 1510382
          Hide
          Felix Meschberger added a comment - - edited
          Show
          Felix Meschberger added a comment - - edited Discussion on the dev list: http://markmail.org/message/2ucwgneqdsiffnbw
          Hide
          Felix Meschberger added a comment -

          Proposed patch against existing bundles (Sling API, Sling JCR API, JCR Base, Jackrabbit Server, JCR Resource Provider, Resource Resovler) and package of the new Service User Mapper bundle (MD5 2e8e5d920e9ce8a175c562d18dc49eed; will be svn cp-ed from http://svn.apache.org/repos/asf/sling/whiteboard/fmeschbe/deprecate_login_administrative/serviceusermapper).

          The notable changes are the introduction of the new ResourceResolverFactory.getServiceResourceresolver and SlingRepository.loginService methods and deprecation of the former administrative login methods.

          In addition the ResourceProviderFactory and SlingRepository services are now registered as service factories to be able to get the calling bundle. This required some refactoring in the AbstractSlingRepository causing extensions to be adapted, too (the registerService method is now final and two methods are added to provide custom registration properties and registration interfaces).

          Show
          Felix Meschberger added a comment - Proposed patch against existing bundles (Sling API, Sling JCR API, JCR Base, Jackrabbit Server, JCR Resource Provider, Resource Resovler) and package of the new Service User Mapper bundle (MD5 2e8e5d920e9ce8a175c562d18dc49eed; will be svn cp-ed from http://svn.apache.org/repos/asf/sling/whiteboard/fmeschbe/deprecate_login_administrative/serviceusermapper ). The notable changes are the introduction of the new ResourceResolverFactory.getServiceResourceresolver and SlingRepository.loginService methods and deprecation of the former administrative login methods. In addition the ResourceProviderFactory and SlingRepository services are now registered as service factories to be able to get the calling bundle. This required some refactoring in the AbstractSlingRepository causing extensions to be adapted, too (the registerService method is now final and two methods are added to provide custom registration properties and registration interfaces).

            People

            • Assignee:
              Felix Meschberger
              Reporter:
              Felix Meschberger
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development