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

refactor: remove CLI login flow #558

Closed
wants to merge 3 commits into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 0 additions & 2 deletions README.rst
Original file line number Diff line number Diff line change
Expand Up @@ -109,8 +109,6 @@ But if you prefer to run directly the docker image here is the list of all envir
+---------------------------------+-----------------------------------------------------------------------------------------------------------------+----------------------------------+
| CLI_CLIENT_SECRET | The client secret for the gateway's CLI client in Keycloak. | dummy-secret |
+---------------------------------+-----------------------------------------------------------------------------------------------------------------+----------------------------------+
| CLI_LOGIN_TIMEOUT | The validity of CLI login nonce in seconds. | 300 |
+---------------------------------+-----------------------------------------------------------------------------------------------------------------+----------------------------------+

Login workflow
--------------
Expand Down
6 changes: 1 addition & 5 deletions app/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -79,11 +79,7 @@
)

url_prefix = app.config["SERVICE_PREFIX"]
blueprints = (
cli_auth.blueprint,
gitlab_auth.blueprint,
web.blueprint,
)
blueprints = (gitlab_auth.blueprint, web.blueprint)


@app.before_request
Expand Down
111 changes: 5 additions & 106 deletions app/auth/cli_auth.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,31 +16,20 @@
# See the License for the specific language governing permissions and
# limitations under the License.
"""Implement Keycloak authentication workflow for CLI."""

import base64
import json
import time
from urllib.parse import urljoin

from flask import Blueprint, current_app, request, session, url_for
from flask import current_app

from .gitlab_auth import GL_SUFFIX
from .oauth_provider_app import KeycloakProviderApp
from .utils import (
get_redis_key_for_cli,
get_redis_key_from_session,
get_redis_key_from_token,
handle_login_request,
handle_token_request,
)

blueprint = Blueprint("cli_auth", __name__, url_prefix="/auth/cli")
from .utils import get_redis_key_from_token

CLI_SUFFIX = "cli_oauth_client"
SCOPE = ["profile", "email", "openid"]


class RenkuCLIGitlabAuthHeaders:
def process(self, request, headers):
@staticmethod
def process(request, headers):
if not request.authorization:
return headers

Expand All @@ -55,93 +44,3 @@ def process(self, request, headers):
headers["Authorization"] = f"Basic {basic_auth}"

return headers


@blueprint.route("/login")
def login():
provider_app = KeycloakProviderApp(
client_id=current_app.config["CLI_CLIENT_ID"],
client_secret=current_app.config["CLI_CLIENT_SECRET"],
base_url=current_app.config["OIDC_ISSUER"],
)
return handle_login_request(
provider_app,
urljoin(current_app.config["HOST_NAME"], url_for("cli_auth.token")),
CLI_SUFFIX,
SCOPE,
)


@blueprint.route("/token")
def token():
response, _ = handle_token_request(request, CLI_SUFFIX)

client_redis_key = get_redis_key_from_session(key_suffix=CLI_SUFFIX)
cli_nonce = session.get("cli_nonce")
if cli_nonce:
server_nonce = session.get("server_nonce")
cli_redis_key = get_redis_key_for_cli(cli_nonce, server_nonce)
login_info = CLILoginInfo(client_redis_key)
current_app.store.set_enc(cli_redis_key, login_info.to_json().encode())
else:
current_app.logger.warn("cli_auth.token called without cli_nonce")

return response


@blueprint.route("/logout")
def logout():
return ""


class CLILoginInfo:
"""Stores some information about CLI login."""

def __init__(self, client_redis_key, login_start=None):
self.client_redis_key = client_redis_key
self.login_start = login_start or time.time()

@classmethod
def from_json(cls, json_string):
"""Create an instance from json string."""
data = json.loads(json_string)
return cls(**data)

def to_json(self):
"""Serialize an instance to json string."""
data = {
"client_redis_key": self.client_redis_key,
"login_start": self.login_start,
}
return json.dumps(data)

def is_expired(self):
elapsed = time.time() - self.login_start
return elapsed > current_app.config["CLI_LOGIN_TIMEOUT"]


def handle_cli_token_request(request):
"""Handle final stage of CLI login."""
cli_nonce = request.args.get("cli_nonce")
server_nonce = request.args.get("server_nonce")
if not cli_nonce or not server_nonce:
return {"error": "Required arguments are missing."}, 400

cli_redis_key = get_redis_key_for_cli(cli_nonce, server_nonce)
current_app.logger.debug(f"Looking up Keycloak for CLI login request {cli_nonce}")
data = current_app.store.get_enc(cli_redis_key)
if not data:
return {"error": "Something went wrong internally."}, 500
current_app.store.delete(cli_redis_key)

login_info = CLILoginInfo.from_json(data.decode())
if login_info.is_expired():
return {"error": "Session expired."}, 403

oauth_client = current_app.store.get_oauth_client(
login_info.client_redis_key, no_refresh=True
)
if not oauth_client:
return {"error": "Access token not found."}, 404

return {"access_token": oauth_client.access_token}
14 changes: 0 additions & 14 deletions app/auth/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,7 @@
# See the License for the specific language governing permissions and
# limitations under the License.

import hashlib
import random
import secrets
import string
from urllib.parse import urljoin

Expand Down Expand Up @@ -69,12 +67,6 @@ def get_redis_key_from_token(token, key_suffix=""):
return _get_redis_key(decoded_token["sub"], key_suffix=key_suffix)


def get_redis_key_for_cli(cli_nonce, server_nonce):
"""Get the redis store from CLI token and user code."""
cli_nonce_hash = hashlib.sha256(cli_nonce.encode()).hexdigest()
return f"cli_{cli_nonce_hash}_{server_nonce}"


def handle_login_request(provider_app, redirect_path, key_suffix, scope):
"""Logic to handle the login requests, avoids duplication"""
oauth_client = RenkuWebApplicationClient(
Expand Down Expand Up @@ -102,9 +94,3 @@ def handle_token_request(request, key_suffix):
)
)
return response, oauth_client


def generate_nonce(n_bits=256):
"""Generate a one-time secure key."""
n_bytes = int(n_bits) // 8
return secrets.token_hex(n_bytes)
63 changes: 14 additions & 49 deletions app/auth/web.py
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
# limitations under the License.
"""Web auth routes."""

import re
from urllib.parse import urljoin

from flask import (
Expand All @@ -30,14 +29,11 @@
url_for,
)

from .cli_auth import CLI_SUFFIX, handle_cli_token_request
from .oauth_provider_app import KeycloakProviderApp
from .utils import (
TEMP_SESSION_KEY,
decode_keycloak_jwt,
generate_nonce,
get_redis_key_from_session,
get_redis_key_from_token,
handle_login_request,
handle_token_request,
)
Expand All @@ -49,37 +45,23 @@


def get_valid_token(headers):
"""Look for a fresh and valid token, first in headers, then in the session."""
authorization = headers.get("Authorization")
authorization_match = (
re.search(r"bearer\s+(?P<token>.+)", authorization, re.IGNORECASE)
if authorization
else None
)

if authorization_match:
renku_token = authorization_match.group("token")
redis_key = get_redis_key_from_token(renku_token, key_suffix=CLI_SUFFIX)
elif headers.get("X-Requested-With") == "XMLHttpRequest" and "sub" in session:
"""Look for a fresh and valid token in the session."""
if headers.get("X-Requested-With") == "XMLHttpRequest" and "sub" in session:
redis_key = get_redis_key_from_session(key_suffix=KC_SUFFIX)
else:
return None

keycloak_oidc_client = current_app.store.get_oauth_client(redis_key)
if hasattr(keycloak_oidc_client, "access_token"):
return keycloak_oidc_client.access_token
current_app.logger.warning(
"The user does not have backend access tokens.",
exc_info=True,
)

keycloak_oidc_client = current_app.store.get_oauth_client(redis_key)
if hasattr(keycloak_oidc_client, "access_token"):
return keycloak_oidc_client.access_token

current_app.logger.warning(
"The user does not have backend access tokens.",
exc_info=True,
)

return None


LOGIN_SEQUENCE = (
"web_auth.login",
"cli_auth.login",
"gitlab_auth.login",
)
LOGIN_SEQUENCE = ("web_auth.login", "gitlab_auth.login")


@blueprint.route("/login/next")
Expand All @@ -92,11 +74,6 @@ def login_next():
redirect_url=urljoin(current_app.config["HOST_NAME"], url_for(next_login)),
)
else:
cli_nonce = session.pop("cli_nonce", None)
if cli_nonce:
server_nonce = session.pop("server_nonce", None)
return render_template("cli_login.html", server_nonce=server_nonce)

return redirect(session["ui_redirect_url"])


Expand All @@ -105,14 +82,7 @@ def login():
"""Log in with Keycloak."""
session.clear()
session["ui_redirect_url"] = request.args.get("redirect_url")

cli_nonce = request.args.get("cli_nonce")
if cli_nonce:
session["cli_nonce"] = cli_nonce
session["server_nonce"] = generate_nonce()
session["login_seq"] = 0
else:
session["login_seq"] = 1
session["login_seq"] = 0

provider_app = KeycloakProviderApp(
client_id=current_app.config["OIDC_CLIENT_ID"],
Expand Down Expand Up @@ -145,11 +115,6 @@ def token():
return response


@blueprint.route("/cli-token")
def info():
return handle_cli_token_request(request)


# @blueprint.route("/user")
# async def user():
# from .. import store
Expand Down
2 changes: 0 additions & 2 deletions app/config.py
Original file line number Diff line number Diff line change
Expand Up @@ -71,8 +71,6 @@
"It is mandatory for CLI login."
)

CLI_LOGIN_TIMEOUT = int(os.environ.get("CLI_LOGIN_TIMEOUT", 300))

GITLAB_URL = os.environ.get("GITLAB_URL", "http://gitlab.renku.build")
GITLAB_CLIENT_ID = os.environ.get("GITLAB_CLIENT_ID", "renku-ui")
GITLAB_CLIENT_SECRET = os.environ.get("GITLAB_CLIENT_SECRET")
Expand Down
28 changes: 0 additions & 28 deletions app/templates/cli_login.html

This file was deleted.

3 changes: 0 additions & 3 deletions app/tests/test_proxy.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,6 @@
import responses

from .. import app
from ..auth.cli_auth import CLI_SUFFIX
from ..auth.gitlab_auth import GL_SUFFIX
from ..auth.oauth_client import RenkuWebApplicationClient
from ..auth.oauth_provider_app import OAuthProviderApp
Expand Down Expand Up @@ -104,8 +103,6 @@ def set_dummy_oauth_client(token, key_suffix):

app.store = OAuthRedis(hex_key=app.config["SECRET_KEY"])

set_dummy_oauth_client(access_token, CLI_SUFFIX)

set_dummy_oauth_client("some_token", GL_SUFFIX)

rv = client.get("/?auth=gitlab", headers=headers)
Expand Down
2 changes: 2 additions & 0 deletions helm-chart/renku-gateway/values.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -268,6 +268,8 @@ traefik:
enabled: false
kubernetesIngress:
enabled: false
rbac:
namespaced: true
resources:
requests:
cpu: "100m"
Expand Down