Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

OAuth2 login process is not stateless #13081

Closed
whatnick opened this issue Dec 15, 2020 · 7 comments · Fixed by #13094
Closed

OAuth2 login process is not stateless #13081

whatnick opened this issue Dec 15, 2020 · 7 comments · Fixed by #13094
Labels
kind:bug This is a clearly a bug

Comments

@whatnick
Copy link
Contributor

whatnick commented Dec 15, 2020

Apache Airflow version: 1.10.14

Kubernetes version (if you are using kubernetes) (use kubectl version): Server Version: version.Info{Major:"1", Minor:"16+", GitVersion:"v1.16.15-eks-ad4801", GitCommit:"ad4801fd44fe0f125c8d13f1b1d4827e8884476d", GitTreeState:"clean", BuildDate:"2020-10-20T23:27:12Z", GoVersion:"go1.13.15", Compiler:"gc", Platform:"linux/amd64"}

Environment:

  • Cloud provider or hardware configuration: AWS / EKS
  • OS (e.g. from /etc/os-release): N/A
  • Kernel (e.g. uname -a): N/A
  • Install tools: N/A
  • Others: N/A

What happened:

Cognito login does not work if second request is not handled by first pod receiving access_token headers.

What you expected to happen:

Logging in via Cognito OAuth2 mode / Code should work via any pod.

How to reproduce it:

Override webserver_config.py with the following code:

"""Default configuration for the Airflow webserver"""
          import logging
          import os
          import json
          from airflow.configuration import conf
          from airflow.www_rbac.security import AirflowSecurityManager
          from flask_appbuilder.security.manager import AUTH_OAUTH

          log = logging.getLogger(__name__)
          basedir = os.path.abspath(os.path.dirname(__file__))

          # The SQLAlchemy connection string.
          SQLALCHEMY_DATABASE_URI = conf.get('core', 'SQL_ALCHEMY_CONN')

          # Flask-WTF flag for CSRF
          WTF_CSRF_ENABLED = True

          CSRF_ENABLED = True
          # ----------------------------------------------------
          # AUTHENTICATION CONFIG
          # ----------------------------------------------------
          # For details on how to set up each of the following authentication, see
          # http://flask-appbuilder.readthedocs.io/en/latest/security.html# authentication-methods
          # for details.

          # The authentication type
          AUTH_TYPE = AUTH_OAUTH

          SECRET_KEY = os.environ.get("FLASK_SECRET_KEY")

          OAUTH_PROVIDERS = [{
              'name': 'aws_cognito',
              'whitelist': ['@ga.gov.au'], 
              'token_key': 'access_token',
              'icon': 'fa-amazon',
              'remote_app': {
                  'api_base_url': os.environ.get("OAUTH2_BASE_URL") + "/",
                  'client_kwargs': {
                      'scope': 'openid email aws.cognito.signin.user.admin'
                  },
                  'authorize_url': os.environ.get("OAUTH2_BASE_URL") + "/authorize",
                  'access_token_url': os.environ.get("OAUTH2_BASE_URL") + "/token",
                  'request_token_url': None,
                  'client_id': os.environ.get("COGNITO_CLIENT_ID"),
                  'client_secret': os.environ.get("COGNITO_CLIENT_SECRET"),
              }
          }]


          class CognitoAirflowSecurityManager(AirflowSecurityManager):
              def oauth_user_info(self, provider, resp):
                  # log.info("Requesting user info from AWS Cognito: {0}".format(resp))
                  assert provider == "aws_cognito"
                  # log.info("Requesting user info from AWS Cognito: {0}".format(resp))
                  me = self.appbuilder.sm.oauth_remotes[provider].get("userInfo")
                  return {
                      "username": me.json().get("username"),
                      "email": me.json().get("email"),
                      "first_name": me.json().get("given_name", ""),
                      "last_name": me.json().get("family_name", ""),
                      "id": me.json().get("sub", ""),
                  }


          SECURITY_MANAGER_CLASS = CognitoAirflowSecurityManager
  • Setup an airflow-app linked a to Cognito user pull and run multiple replicas of the airflow-web pod.
  • Login will start failing and work may be 1 in 9 attempts.

Anything else we need to know:

There are 3 possible work arounds using infrastructure changes instead of airflow-web code changes.

  • Use a single pod for airflow-web to avoid session issues
  • Make ALB sticky via ingress to give users the same pod consistently
  • Sharing the same secret key across all airflow-web pods using the environment
@whatnick whatnick added the kind:bug This is a clearly a bug label Dec 15, 2020
@boring-cyborg
Copy link

boring-cyborg bot commented Dec 15, 2020

Thanks for opening your first issue here! Be sure to follow the issue template!

@pindge
Copy link

pindge commented Dec 15, 2020

Note, this is running v1.10.14 with flask appbuilder 3.1.1 in web instance

      extraPipPackages:
      - "authlib"
      - "Flask-AppBuilder==3.1.1"

When the secret key is not specified and is randomly generated, each web instance has a different secret key which breaks the login process when the serving pod is switched.

@ashb
Copy link
Member

ashb commented Dec 15, 2020

Sharing the same secret key across all airflow-web pods using the environment

That is the correct fix, and not a work around -- it is you something you should do in your config.

Does that fix the problem, if so then the only change we need to make is likely a doc one.

@whatnick
Copy link
Contributor Author

Yes this fix resolves the problem. Happy to make a doc change in the OAuth2 page and PR.

@ashb
Copy link
Member

ashb commented Dec 15, 2020

Yes please @whatnick !

@pindge
Copy link

pindge commented Dec 15, 2020

This potentially can be addressed like how airflow-helm-chart handles fernet key

{{- if not .Values.fernetKeySecretName }}
{{ $generated_fernet_key := (randAlphaNum 32 | b64enc) }}
kind: Secret
apiVersion: v1
metadata:
name: {{ .Release.Name }}-fernet-key
labels:
release: {{ .Release.Name }}
chart: {{ .Chart.Name }}
heritage: {{ .Release.Service }}
{{- with .Values.labels }}
{{ toYaml . | indent 4 }}
{{- end }}
annotations:
"helm.sh/hook": "pre-install"
"helm.sh/hook-delete-policy": "before-hook-creation"
"helm.sh/hook-weight": "0"
type: Opaque
data:
fernet-key: {{ (default $generated_fernet_key .Values.fernetKey) | b64enc | quote }}
{{- end }}

@ashb
Copy link
Member

ashb commented Dec 15, 2020

This potentially can be addressed like how airflow-helm-chart handles fernet key

{{- if not .Values.fernetKeySecretName }}
{{ $generated_fernet_key := (randAlphaNum 32 | b64enc) }}
kind: Secret
apiVersion: v1
metadata:
name: {{ .Release.Name }}-fernet-key
labels:
release: {{ .Release.Name }}
chart: {{ .Chart.Name }}
heritage: {{ .Release.Service }}
{{- with .Values.labels }}
{{ toYaml . | indent 4 }}
{{- end }}
annotations:
"helm.sh/hook": "pre-install"
"helm.sh/hook-delete-policy": "before-hook-creation"
"helm.sh/hook-weight": "0"
type: Opaque
data:
fernet-key: {{ (default $generated_fernet_key .Values.fernetKey) | b64enc | quote }}
{{- end }}

Also a good idea

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
kind:bug This is a clearly a bug
Projects
None yet
Development

Successfully merging a pull request may close this issue.

3 participants