Re: [PR] Re-work JWT Validation and Generation to use public/private key and official claims [airflow]

["vincbeck (via GitHub)" <gi...@apache.org>]

vincbeck commented on code in PR #46981:
URL: https://github.com/apache/airflow/pull/46981#discussion_r1989909399


##########
airflow/api_fastapi/auth/managers/base_auth_manager.py:
##########
@@ -84,25 +84,29 @@ def deserialize_user(self, token: dict[str, Any]) -> T:
         """Create a user object from dict."""
 
     @abstractmethod
-    def serialize_user(self, user: T) -> dict[str, Any]:
-        """Create a dict from a user object."""
+    def serialize_user(self, user: T) -> tuple[str, dict[str, Any]]:
+        """Create a subject and extra claims dict from a user object."""
 
     def get_user_from_token(self, token: str) -> BaseUser:
         """Verify the JWT token is valid and create a user object from it if valid."""
         try:
-            payload: dict[str, Any] = self._get_token_signer().verify_token(token)
+            payload: dict[str, Any] = self._get_token_validator().validated_claims(token)
             return self.deserialize_user(payload)
         except InvalidTokenError as e:
             log.error("JWT token is not valid")
             raise e
 
-    def get_jwt_token(
+    def generate_jwt(
         self, user: T, *, expiration_time_in_seconds: int = conf.getint("api", "auth_jwt_expiration_time")
     ) -> str:
         """Return the JWT token from a user object."""
-        return self._get_token_signer(
-            expiration_time_in_seconds=expiration_time_in_seconds
-        ).generate_signed_token(self.serialize_user(user))
+        return self._get_token_signer(expiration_time_in_seconds=expiration_time_in_seconds).generate(
+            *self.serialize_user(user)
+        )
+
+    @abstractmethod
+    def is_logged_in(self) -> bool:
+        """Return whether the user is logged in."""

Review Comment:
   This method has been removed and should not be added back



##########
airflow/api_fastapi/auth/managers/base_auth_manager.py:
##########
@@ -414,16 +418,30 @@ def get_fastapi_app(self) -> FastAPI | None:
     @staticmethod
     def _get_token_signer(
         expiration_time_in_seconds: int = conf.getint("api", "auth_jwt_expiration_time"),
-    ) -> JWTSigner:
+    ) -> JWTGenerator:
         """
         Return the signer used to sign JWT token.
 
         :meta private:
 
         :param expiration_time_in_seconds: expiration time in seconds of the token
         """
-        return JWTSigner(
-            secret_key=get_signing_key("api", "auth_jwt_secret"),
-            expiration_time_in_seconds=expiration_time_in_seconds,
+        return JWTGenerator(
+            secret_key=get_signing_key("api_auth", "jwt_secret"),
+            valid_for=expiration_time_in_seconds,
+            audience="front-apis",
+        )
+
+    @staticmethod
+    def _get_token_validator() -> JWTValidator:
+        """
+        Return the signer used to sign JWT token.
+
+        :meta private:
+        """
+        return JWTValidator(
+            # issuer=conf.get("api_auth", "jwt_iussuer"),
+            secret_key=get_signing_key("api_auth", "jwt_secret"),
+            leeway=conf.getint("api", "auth_jwt_expiration_time"),
             audience="front-apis",

Review Comment:
   Should we then use the new config `auth_jwt_audience`?



##########
airflow/config_templates/config.yml:
##########
@@ -1443,36 +1443,112 @@ api:
       version_added: 2.7.0
       example: ~
       default: "False"
-    auth_jwt_secret:
-      description: |
-        Secret key used to encode and decode JWT tokens to authenticate to public and private APIs.
-        It should be as random as possible. However, when running more than 1 instances of API services,
-        make sure all of them use the same ``jwt_secret`` otherwise calls will fail on authentication.
+api_auth:
+  description: Settings relating to authentication on the Airflow APIs
+  options:
+    jwt_audience:
       version_added: 3.0.0
+      description: |
+        The audience claim to use when generating and validating JWTs for the API.
+
+        This variable can be a single value, or a comma-separated string, in which case the first value is the
+        one that will be used when generating, and the others are accepted at validation time.
+
+        Not required, but strongly encouraged
+      example: "my-unique-airflow-id"
+      default: ~
       type: string
-      sensitive: true
-      example: ~
-      default: "{JWT_SECRET_KEY}"
-    auth_jwt_expiration_time:
+    jwt_expiration_time:
       description: |
         Number in seconds until the JWT token used for authentication expires. When the token expires,
         all API calls using this token will fail on authentication.
+
         Make sure that time on ALL the machines that you run airflow components on is synchronized
         (for example using ntpd) otherwise you might get "forbidden" errors.
       version_added: 3.0.0
       type: integer
       example: ~
       default: "86400"
-    auth_jwt_cli_expiration_time:
+    jwt_cli_expiration_time:
       description: |
         Number in seconds until the JWT token used for authentication expires for CLI commands.
         When the token expires, all CLI calls using this token will fail on authentication.
+
         Make sure that time on ALL the machines that you run airflow components on is synchronized
         (for example using ntpd) otherwise you might get "forbidden" errors.
       version_added: 3.0.0
       type: integer
       example: ~
       default: "3600"
+    jwt_secret:
+      description: |
+        Secret key used to encode and decode JWT tokens to authenticate to public and private APIs.
+
+        It should be as random as possible. However, when running more than 1 instances of API services,
+        make sure all of them use the same ``jwt_secret`` otherwise calls will fail on authentication.
+
+        Mutually exclusive with ``jwt_private_key_path``.
+      version_added: 3.0.0
+      type: string
+      sensitive: true
+      example: ~
+      default: "{JWT_SECRET_KEY}"
+    jwt_private_key_path:
+      version_added: 3.0.0
+      description: |
+        The path to a file containing a PEM-encoded private key use when generating Task Identity tokens in
+        the executor.
+
+        Mutually exclusive with ``jwt_secret``.
+      default: ~
+      example: /path/to/private_key.pem
+      type: string
+    jwt_algorithm:
+      version_added: 3.0.0
+      description: |
+        The algorithm name use when generating and validating JWT Task Identities.
+
+        This value must be appropriate for the given private key type.
+
+        Default is "HS512" if ``jwt_secret`` is set, or "EdDSA" otherwise
+      example: '"EdDSA" or "HS512"'
+      type: string
+      default: ~
+    trusted_jwks_url:
+      version_added: 3.0.0
+      description: |
+        The public signing keys of Task Execution token issuers to trust. It must contain the public key
+        related to ``jwt_private_key_path`` else tasks will be unlikely to execute successfully.
+
+        Can be a local file path (without the ``file://`` url) or an http or https URL.
+
+        If a remote URL is given it will be polled periodically for changes.

Review Comment:
   Mutually exclusive with ``jwt_secret``?



-- 
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.

To unsubscribe, e-mail: commits-unsubscribe@airflow.apache.org

For queries about this service, please contact Infrastructure at:
users@infra.apache.org