Skip to content

ITP-2120 add options for setting public key and add code for validati…#3

Open
perekatypola wants to merge 6 commits intomasterfrom
feature/ITP-2120_add_oauth2_token_check
Open

ITP-2120 add options for setting public key and add code for validati…#3
perekatypola wants to merge 6 commits intomasterfrom
feature/ITP-2120_add_oauth2_token_check

Conversation

@perekatypola
Copy link
Collaborator

…ng oauth2_proxy token

Description

Please include a summary of the change and which issue is fixed. Please also include relevant motivation and context. List any dependencies that are required for this change.

Fixes # (issue)

Type of change

Please delete options that are not relevant.

  • Documentation Change
  • Bug fix (non-breaking change which fixes an issue)
  • New feature (non-breaking change which adds functionality)
  • Breaking change (fix or feature that would cause existing functionality to not work as expected)
  • This change requires a documentation update

Checklist:

  • I have performed a self-review of my own code
  • I have made corresponding changes to the documentation
  • My changes generate no new warnings
  • I have run tests (pytest) that prove my fix is effective or that my feature works
  • I have updated the CHANGELOG.md file accordingly
  • I have added tests that prove my fix is effective or that my feature works

Copy link
Collaborator

@gnomeby gnomeby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Add OAUTH2.md help file

disable_delete,
verbose,
json,
oidc_public_key
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oauth2_token_verification_key

logging.warning(encoded_jwt)
if encoded_jwt:
base64_key_clean = os.environ.get('OIDC_PUBLIC_KEY').replace('\n', '').replace('\r', '').replace(' ', '')
key_formatted = textwrap.fill(base64_key_clean, 64)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

форматирование лишнее

'-----END PUBLIC KEY-----'
)
payload = jwt.decode(encoded_jwt, key)
logging.warning(payload)
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

лишнее логирование

return "<h1>403 Not authorized (oauth2_proxy)</h1>", 403
except errors.JoseError as ex:
logging.warning(ex)
return "<h1>403 Not authorized (oauth2_proxy)</h1>", 403
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[403 Forbidden]

payload.validate()
return None
else:
return "<h1>403 Not authorized (oauth2_proxy)</h1>", 403
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

[400 Bad Request]

@blueprint.before_request
def basic_http_auth():
try:
encoded_jwt = request.headers.get('X-Forwarded-Access-Token')
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Должен быть настраиваемый заголовок

setup.py Outdated
platforms='any',
install_requires=['rq>=1.0', 'Flask', 'redis', 'arrow', 'redis-sentinel-url'],
extras_require={
'oauth2_proxy': [
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oauth2

else:
add_oauth2_token_validation()
else:
if WITH_OAUTH:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

эта секция лишняя

"-j", "--json", is_flag=True, default=False, help="Enable JSONSerializer"
)
@click.option(
"--oidc-public-key", default=None, help="Public key for OIDC provider (needed if ouath2_proxy is used)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Verification key for oauth2 access_token verification (when oauth2 provider is used as middleware or reverse proxy. For example: oauth2_proxy+KeyCloak)

Copy link
Collaborator

@gnomeby gnomeby left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Добавить заметку в README.md о том, что есть опция валидировать oauth2 token

"-j", "--json", is_flag=True, default=False, help="Enable JSONSerializer"
)
@click.option(
"--oauth2-token-verification-key", default=None, help="Verification key for oauth2 access_token verification (when oauth2 provider is used as middleware or reverse proxy. For example: oauth2_proxy+KeyCloak)"
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

oauth2 extension is required

app.config['OAUTH2_TOKEN_VERIFICATION_HEADER'] = oauth2_token_verification_header

if app.config['OAUTH2_TOKEN_VERIFICATION_KEY'] and app.config['OAUTH2_TOKEN_VERIFICATION_HEADER']:
if not WITH_OAUTH:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Нужно инвертировать условие. Надо стараться делать условия позитивными

@@ -0,0 +1,19 @@
## Installation of oauth2 extension:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • Working with oauth2_token


restart: always

oauth2-proxy:
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Рассмотреть возможно добавить сюда KEyCloak который при старте сетатип нужного клиента. С тем чтобы было просто достаточно запустить docker compose и сразу работало.

&& rm -rf /var/lib/apt/lists/*

ADD . /
RUN pip3 install '.[oauth2]'
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Просто поменять основной Dockerfile

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants