Skip to content

Commit

Permalink
fixup! Composer RBAC/per-folder role autoregistration patch
Browse files Browse the repository at this point in the history
Do not require [webserver]google_oauth2_audience property when loading
Composer security manager implementation. The property is not always
used and some Composer versions don't provide it.

Internal bug

Change-Id: Ib0b031f8f3791163a5492fe0ac8e4fae20f0ab60
(cherry picked from commit ae325bf4b038104a42b380e3fa8c8f805f56a06c)
GitOrigin-RevId: eafe7de271b260980ea685bd1fb618b790acb077
  • Loading branch information
Cloud Composer Team committed Jan 30, 2023
1 parent df4e992 commit 8690cfc
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 7 deletions.
5 changes: 1 addition & 4 deletions airflow/composer/security_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -33,9 +33,6 @@

log = logging.getLogger(__file__)

# Expected audience of IAP JWT.
IAP_JWT_AUDIENCE = conf.get("webserver", "google_oauth2_audience")


def _decode_iap_jwt(iap_jwt):
"""Returns username and email decoded from the given IAP JWT.
Expand All @@ -52,7 +49,7 @@ def _decode_iap_jwt(iap_jwt):
decoded_jwt = id_token.verify_token(
iap_jwt,
requests.Request(),
audience=IAP_JWT_AUDIENCE,
audience=conf.get("webserver", "google_oauth2_audience"),
certs_url="https://www.gstatic.com/iap/verify/public_key",
)
return decoded_jwt["sub"], decoded_jwt["email"]
Expand Down
3 changes: 2 additions & 1 deletion tests/composer/api/backend/test_composer_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -41,7 +41,6 @@ def setUpClass(cls):
shutil.copy(cls.COMPOSER_WEBSERVER_CONFIG, WEBSERVER_CONFIG)
with conf_vars(
{
("webserver", "google_oauth2_audience"): "audience",
("webserver", "rbac_user_registration_role"): "Viewer",
("api", "auth_backends"): "airflow.composer.api.backend.composer_auth",
("api", "composer_auth_user_registration_role"): "User",
Expand All @@ -57,6 +56,7 @@ def tearDownClass(cls):
shutil.copy(cls.WEBSERVER_CONFIG_BACKUP, WEBSERVER_CONFIG)

@mock.patch("airflow.composer.security_manager._decode_iap_jwt", autospec=True)
@conf_vars({("webserver", "google_oauth2_audience"): "audience"})
def test_authentication_success(self, _decode_iap_jwt_mock):
def _decode_iap_jwt_mock_side_effect(iap_jwt):
assert iap_jwt == "jwt-test"
Expand All @@ -80,6 +80,7 @@ def _decode_iap_jwt_mock_side_effect(iap_jwt):
# "User" role doesn't have access to pools endpoint.
assert pools_response.status_code == 403

@conf_vars({("webserver", "google_oauth2_audience"): "audience"})
def test_authentication_failure(self):
response = self.test_client.get(
"/api/v1/pools", headers={"X-Goog-IAP-JWT-Assertion": "invalid-jwt-token"}
Expand Down
3 changes: 2 additions & 1 deletion tests/composer/test_security_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@ def setUpClass(cls):
shutil.copy(cls.COMPOSER_WEBSERVER_CONFIG, WEBSERVER_CONFIG)
with conf_vars(
{
("webserver", "google_oauth2_audience"): "audience",
("webserver", "rbac_user_registration_role"): "Viewer",
("webserver", "rbac_autoregister_per_folder_roles"): "True",
}
Expand Down Expand Up @@ -71,6 +70,7 @@ def test_login_incorrect_jwt(self):
assert resp.status_code == 403

@mock.patch("airflow.composer.security_manager.id_token", autospec=True)
@conf_vars({("webserver", "google_oauth2_audience"): "audience"})
def test_login_user_auto_registered(self, id_token_mock):
username = f"test-{self.get_random_id()}"
email = f"test-{self.get_random_id()}@test.com"
Expand Down Expand Up @@ -149,6 +149,7 @@ def id_token_mock_verify_token_side_effect(
assert resp.status_code == 403

@mock.patch("airflow.composer.security_manager.id_token", autospec=True)
@conf_vars({("webserver", "google_oauth2_audience"): "audience"})
def test_login_user_preregistered(self, id_token_mock):
username = f"test-{self.get_random_id()}"
email = f"test-{self.get_random_id()}@test.com"
Expand Down
1 change: 0 additions & 1 deletion tests/composer/test_webserver_config.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@ class TestWebserverConfig(unittest.TestCase):
def test_webserver_config(self):
with conf_vars(
{
("webserver", "google_oauth2_audience"): "audience",
("webserver", "rbac_user_registration_role"): "Viewer",
}
):
Expand Down

0 comments on commit 8690cfc

Please sign in to comment.