Uploaded image for project: 'Request For Comments'
  1. Request For Comments
  2. RFC-487

Add PyJWT and "cryptography" dependencies for DAX

    XMLWordPrintable

    Details

    • Type: RFC
    • Status: Retired
    • Resolution: Done
    • Component/s: DM
    • Labels:
      None

      Description

      In order to support JSON Web Tokens, a form of authentication token given by OAuth 2/OpenID Connect providers, in our DAX Python web applications, we need library support for parsing and validating JWTs, and associated underlying support for verifying cryptographic signatures.

      This RFC is to add PyJWT (https://github.com/jpadilla/pyjwt) and Cryptography (https://cryptography.io/en/latest/) to eups for use in dax_webserv builds via pip install in the python environment.

        Attachments

          Issue Links

            Activity

            No builds found.
            bvan Brian Van Klaveren created issue -
            bvan Brian Van Klaveren made changes -
            Field Original Value New Value
            Link This issue relates to DM-4995 [ DM-4995 ]
            bvan Brian Van Klaveren made changes -
            Risk Score 0
            jhoblitt Joshua Hoblitt made changes -
            Watchers Alexander Withers, Brian Van Klaveren, Fritz Mueller, Kenny Lo [ Alexander Withers, Brian Van Klaveren, Fritz Mueller, Kenny Lo ] Alexander Withers, Brian Van Klaveren, Fritz Mueller, Gabriele Comoretto, Joshua Hoblitt, Kenny Lo [ Alexander Withers, Brian Van Klaveren, Fritz Mueller, Gabriele Comoretto, Joshua Hoblitt, Kenny Lo ]
            Hide
            ktl Kian-Tat Lim added a comment -

            Can you say a little bit about why these two in particular?  Also, why add to eups as opposed to installing in another way?

            Show
            ktl Kian-Tat Lim added a comment - Can you say a little bit about why these two in particular?  Also, why add to eups as opposed to installing in another way?
            Hide
            bvan Brian Van Klaveren added a comment -

            They are the de-facto standards in Python and they are also used in SciTokens:
            https://github.com/scitokens/scitokens/blob/master/requirements.txt

            I don't care if it's in eups really, but I figured versions of security libraries in particular should probably be tracked by a system, otherwise how do you know what you might need to fix if there's a critical vulnerability?

            I also realize this is new territory for the stack, I didn't want to go too far down that philosophical hole.

            Show
            bvan Brian Van Klaveren added a comment - They are the de-facto standards in Python and they are also used in SciTokens: https://github.com/scitokens/scitokens/blob/master/requirements.txt I don't care if it's in eups really, but I figured versions of security libraries in particular should probably be tracked by a system, otherwise how do you know what you might need to fix if there's a critical vulnerability? I also realize this is new territory for the stack, I didn't want to go too far down that philosophical hole.
            Hide
            jhoblitt Joshua Hoblitt added a comment -

            I don't think an RFC, conda environment file change, and/or EUPS stub package is needed in this case as dax is not part of lsst_distrib/a science-pipeline release.

            My recommendation is to treat dax_webserv as a typical python package and convert it to use setuptools. I believe all of the dependencies, minus dax_imgserv, could be declared in setup.py with a stub requirements.txt. Eg.,

            --index-url https://pypi.python.org/simple/
             
            -e .
            

            It could be kept as an eups product with an eupspkg.cfg.sh but, after a quick glance, I would lean towards relying on a docker image to provide dax_imgserv.

            Show
            jhoblitt Joshua Hoblitt added a comment - I don't think an RFC, conda environment file change, and/or EUPS stub package is needed in this case as dax is not part of lsst_distrib /a science-pipeline release. My recommendation is to treat dax_webserv as a typical python package and convert it to use setuptools . I believe all of the dependencies, minus dax_imgserv , could be declared in setup.py with a stub requirements.txt . Eg., --index-url https: //pypi.python.org/simple/   -e . It could be kept as an eups product with an eupspkg.cfg.sh but, after a quick glance, I would lean towards relying on a docker image to provide dax_imgserv .
            Hide
            tjenness Tim Jenness added a comment -

            In theory, an RFC is required when new external dependencies are added, even if those are on PyPI and the outcome of the RFC is the addition of one line to a requirements.txt file.

            Show
            tjenness Tim Jenness added a comment - In theory, an RFC is required when new external dependencies are added, even if those are on PyPI and the outcome of the RFC is the addition of one line to a requirements.txt file.
            Hide
            swinbank John Swinbank added a comment -

            While I don't see any real objections above, I do note that Kian-Tat Lim hasn't followed up on the response to his questions. Kian-Tat Lim, do you have any thoughts that would help Brian Van Klaveren close this RFC?

            Show
            swinbank John Swinbank added a comment - While I don't see any real objections above, I do note that Kian-Tat Lim hasn't followed up on the response to his questions. Kian-Tat Lim , do you have any thoughts that would help Brian Van Klaveren close this RFC?
            Hide
            ktl Kian-Tat Lim added a comment -

            I am fine with the justification given for the selection of these packages.

            It's not clear to me if these packages are needed by imgserv directly or if they can be used only at the webserv level. If the latter, I definitely think eups packaging should be avoided. If imgserv needs access directly in its code, then eups packaging may be inevitable.

            Show
            ktl Kian-Tat Lim added a comment - I am fine with the justification given for the selection of these packages. It's not clear to me if these packages are needed by imgserv directly or if they can be used only at the webserv level. If the latter, I definitely think eups packaging should be avoided. If imgserv needs access directly in its code, then eups packaging may be inevitable.
            Hide
            bvan Brian Van Klaveren added a comment -

            I think I'm fine with it being installed it manually via the Dockerfile at container, post-eups-install time, via pip install.

            Show
            bvan Brian Van Klaveren added a comment - I think I'm fine with it being installed it manually via the Dockerfile at container, post-eups-install time, via pip install.
            bvan Brian Van Klaveren made changes -
            Summary Add PyJWT and "cryptography" dependencies for DAX into EUPS Add PyJWT and "cryptography" dependencies for DAX
            bvan Brian Van Klaveren made changes -
            Description In order to support JSON Web Tokens, a form of authentication token given by OAuth 2/OpenID Connect providers, in our DAX Python web applications, we need library support for parsing and validating JWTs, and associated underlying support for verifying cryptographic signatures.

            This RFC is to add PyJWT (https://github.com/jpadilla/pyjwt) and Cryptography (https://cryptography.io/en/latest/) to eups for use in dax_webserv builds.
            In order to support JSON Web Tokens, a form of authentication token given by OAuth 2/OpenID Connect providers, in our DAX Python web applications, we need library support for parsing and validating JWTs, and associated underlying support for verifying cryptographic signatures.

            This RFC is to add PyJWT (https://github.com/jpadilla/pyjwt) and Cryptography (https://cryptography.io/en/latest/) -to eups- for use in dax_webserv builds.
            bvan Brian Van Klaveren made changes -
            Description In order to support JSON Web Tokens, a form of authentication token given by OAuth 2/OpenID Connect providers, in our DAX Python web applications, we need library support for parsing and validating JWTs, and associated underlying support for verifying cryptographic signatures.

            This RFC is to add PyJWT (https://github.com/jpadilla/pyjwt) and Cryptography (https://cryptography.io/en/latest/) -to eups- for use in dax_webserv builds.
            In order to support JSON Web Tokens, a form of authentication token given by OAuth 2/OpenID Connect providers, in our DAX Python web applications, we need library support for parsing and validating JWTs, and associated underlying support for verifying cryptographic signatures.

            This RFC is to add PyJWT (https://github.com/jpadilla/pyjwt) and Cryptography (https://cryptography.io/en/latest/) -to eups- for use in dax_webserv builds via pip install in the python environment.
            bvan Brian Van Klaveren made changes -
            Status Proposed [ 10805 ] Adopted [ 10806 ]
            xiuqin Xiuqin Wu [X] (Inactive) made changes -
            Link This issue relates to DM-14501 [ DM-14501 ]
            Hide
            tjenness Tim Jenness added a comment -

            Brian Van Klaveren please add at least one triggering ticket to this RFC. I have no way of knowing when this RFC has been implemented without this extra information.

            Show
            tjenness Tim Jenness added a comment - Brian Van Klaveren please add at least one triggering ticket to this RFC. I have no way of knowing when this RFC has been implemented without this extra information.
            Hide
            kennylo Kenny Lo added a comment - - edited

            DM-14193 for  imgserv  is definitely one triggering ticket.

            Show
            kennylo Kenny Lo added a comment - - edited DM-14193 for  imgserv   is definitely one triggering ticket.
            tjenness Tim Jenness made changes -
            Link This issue is triggering DM-14193 [ DM-14193 ]
            Hide
            tjenness Tim Jenness added a comment -

            Added. You can add more if you have them.

            Show
            tjenness Tim Jenness added a comment - Added. You can add more if you have them.
            Hide
            ktl Kian-Tat Lim added a comment -

            The triggered tickets should be ones that implement the RFC, not ones that use the result of implementing the RFC. I don't think that DM-14193 qualifies. Brian Van Klaveren must add another.

            Show
            ktl Kian-Tat Lim added a comment - The triggered tickets should be ones that implement the RFC, not ones that use the result of implementing the RFC. I don't think that DM-14193 qualifies. Brian Van Klaveren must add another.
            bvan Brian Van Klaveren made changes -
            Link This issue is triggering DM-15395 [ DM-15395 ]
            Hide
            bvan Brian Van Klaveren added a comment -

            We've talked this over, and we'd like to add cryptography and PyJWT to the lsst conda dependencies for now.

            We will plan on rewriting the DAX services to consume a stack container or perform a binary install and then, using setuptools, install in that environment, but that will require several different things to implement properly, including a change in CI.

            I've created a triggering issue, and a follow up issue to convert DAX services to a setuptools-based system (with the goal of removing the conda dependencies as well)

            Show
            bvan Brian Van Klaveren added a comment - We've talked this over, and we'd like to add cryptography and PyJWT to the lsst conda dependencies for now. We will plan on rewriting the DAX services to consume a stack container or perform a binary install and then, using setuptools, install in that environment, but that will require several different things to implement properly, including a change in CI. I've created a triggering issue, and a follow up issue to convert DAX services to a setuptools-based system (with the goal of removing the conda dependencies as well)
            Hide
            bvan Brian Van Klaveren added a comment -

            The DAX APIs have changed how they consume the stack and work with dependencies, in part because of the lack of consensus on how to handle this issue. The DAX APIs now consume a stack docker image and layer on top the dependencies they need via a requirements.txt file.

            Show
            bvan Brian Van Klaveren added a comment - The DAX APIs have changed how they consume the stack and work with dependencies, in part because of the lack of consensus on how to handle this issue. The DAX APIs now consume a stack docker image and layer on top the dependencies they need via a requirements.txt file.
            bvan Brian Van Klaveren made changes -
            Link This issue relates to DM-19276 [ DM-19276 ]
            bvan Brian Van Klaveren made changes -
            Resolution Won't Do [ 10101 ] Done [ 10000 ]
            Status Adopted [ 10806 ] Retired [ 10705 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 20413 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 20452 ]
            gcomoretto Gabriele Comoretto [X] (Inactive) made changes -
            Remote Link This issue links to "Page (Confluence)" [ 20452 ]

              People

              Assignee:
              bvan Brian Van Klaveren
              Reporter:
              bvan Brian Van Klaveren
              Watchers:
              Alexander Withers [X] (Inactive), Brian Van Klaveren, Fritz Mueller, Gabriele Comoretto [X] (Inactive), John Swinbank, Joshua Hoblitt, Kenny Lo, Kian-Tat Lim, Tim Jenness, Xiuqin Wu [X] (Inactive)
              Votes:
              0 Vote for this issue
              Watchers:
              10 Start watching this issue

                Dates

                Created:
                Updated:
                Resolved:
                Planned End:

                  CI Builds

                  No builds found.