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

Update django-oauth-toolkit #2710

Closed
phillxnet opened this issue Oct 17, 2023 · 8 comments
Closed

Update django-oauth-toolkit #2710

phillxnet opened this issue Oct 17, 2023 · 8 comments
Assignees
Milestone

Comments

@phillxnet
Copy link
Member

phillxnet commented Oct 17, 2023

Our current pinning relates to a now defunct Django and Python constraint. It is proposed that we remove this pinning as from the projects PyPi page (below) we can now use the latest available.

PyPi: https://pypi.org/project/django-oauth-toolkit/
Changelog: https://github.com/jazzband/django-oauth-toolkit/blob/master/CHANGELOG.md

The Changelog notes a number of breaking changes between our current version and the latest.

A likely related pinning that should be addressed also within this issue.

oauthlib = "==3.1.0"  # Last Python 2.7 compat + 3.7 compat.
@phillxnet phillxnet added this to the Django 3.2 milestone Oct 17, 2023
@phillxnet phillxnet self-assigned this Oct 25, 2023
@phillxnet
Copy link
Member Author

lbuildvm:/opt/rockstor # poetry show --tree
...
django-oauth-toolkit 1.1.2 OAuth2 Provider for Django
├── django >=1.11
│   ├── pytz * 
│   └── sqlparse >=0.2.2 
├── oauthlib >=2.0.3
└── requests >=2.13.0
    ├── certifi >=2017.4.17 
    ├── charset-normalizer >=2,<4 
    ├── idna >=2.5,<4 
    └── urllib3 >=1.21.1,<3 
...
oauthlib 3.1.0 A generic, spec-compliant, thorough implementation of the OAuth request-signing logic

@phillxnet
Copy link
Member Author

Removing our spec for oauthlib, as it is already a hard dependency of django-oauth-toolkit, and unpinning the later; we have:

lbuildvm:/opt/rockstor # poetry update
Updating dependencies
Resolving dependencies... (4.2s)

Writing lock file

Package operations: 6 installs, 4 updates, 0 removals

  • Installing pycparser (2.21)
  • Installing cffi (1.16.0)
  • Installing wrapt (1.15.0)
  • Updating charset-normalizer (3.3.0 -> 3.3.1)
  • Installing cryptography (41.0.5)
  • Installing deprecated (1.2.14)
  • Updating greenlet (3.0.0 -> 3.0.1)
  • Installing jwcrypto (1.5.0)
  • Updating oauthlib (3.1.0 -> 3.2.2)
  • Updating django-oauth-toolkit (1.1.2 -> 2.3.0)

With the following updated django-oauth-toolkit dependencies:

django-oauth-toolkit 2.3.0 OAuth2 Provider for Django
├── django >=2.2,<4.0.0 || >4.0.0
│   ├── pytz * 
│   └── sqlparse >=0.2.2 
├── jwcrypto >=0.8.0
│   ├── cryptography >=3.4 
│   │   └── cffi >=1.12 
│   │       └── pycparser * 
│   └── deprecated * 
│       └── wrapt >=1.10,<2 
├── oauthlib >=3.1.0
└── requests >=2.13.0
    ├── certifi >=2017.4.17 
    ├── charset-normalizer >=2,<4 
    ├── idna >=2.5,<4 
    └── urllib3 >=1.21.1,<3 

@phillxnet
Copy link
Member Author

We have the following incidental updates here:

  • Updating charset-normalizer (3.3.0 -> 3.3.1)
  • Updating greenlet (3.0.0 -> 3.0.1)

@phillxnet
Copy link
Member Author

phillxnet commented Oct 25, 2023

Upon starting our services we have, in rockstor.log (under debug mode):

...

[25/Oct/2023 16:38:51] DEBUG [scripts.initrock:567] migrate (--fake-initial) contenttypes
[25/Oct/2023 16:38:51] DEBUG [system.osi:235] Running command: /opt/rockstor/.venv/bin/django-admin migrate --noinput --fake-initial --database=default contenttypes
...
[25/Oct/2023 16:38:57] DEBUG [system.osi:235] Running command: /opt/rockstor/.venv/bin/django-admin migrate --noinput auth
...
[25/Oct/2023 16:38:59] DEBUG [system.osi:235] Running command: /opt/rockstor/.venv/bin/django-admin migrate --noinput storageadmin
...
[25/Oct/2023 16:39:01] DEBUG [system.osi:235] Running command: /opt/rockstor/.venv/bin/django-admin migrate --noinput --database=smart_manager smart_manager
...
[25/Oct/2023 16:39:05] DEBUG [scripts.initrock:610] The 0002_08_updates migration is not already applied so fake apply it now
[25/Oct/2023 16:39:05] DEBUG [system.osi:235] Running command: /opt/rockstor/.venv/bin/django-admin migrate --noinput --fake oauth2_provider 0002_08_updates
...
[25/Oct/2023 16:39:06] ERROR [system.osi:261] non-zero code(1) returned by command: ['/opt/rockstor/.venv/bin/django-admin', 'migrate', '--noinput', '--fake', 'oauth2_provider', '0002_08_updates']. output: [''] error: ["CommandError: Cannot find a migration matching '0002_08_updates' from app 'oauth2_provider'.", '']
...

Likely what we are seeing here is the result of an upstream migrations squash:

https://github.com/jazzband/django-oauth-toolkit/blob/master/oauth2_provider/migrations/0001_initial.py

https://github.com/jazzband/django-oauth-toolkit/blob/4c1367942fcce5a8cb36d7e5fa4e39d481a361cc/oauth2_provider/migrations/0001_initial.py#L10-L22

"""
The following migrations are squashed here:
- 0001_initial.py
- 0002_08_updates.py
- 0003_auto_20160316_1503.py
- 0004_auto_20160525_1623.py
- 0005_auto_20170514_1141.py
- 0006_auto_20171214_2232.py
"""

Which may (hopefully) render our prior patch re fake applying of 0002_08_updates redundant.

@phillxnet
Copy link
Member Author

phillxnet commented Oct 25, 2023

Referencing our prior encounter with changes in this area, and our adaptation to them:

From the above issue we have @FroggyFlox excellent synopsis:

The migrations for this app are part of the django_oauth_toolkit that was upgraded to a newer version in #2365. This newer version includes the following migrations:

0001_initial.py
0002_08_updates.py
0003_auto_20160316_1503.py
0004_auto_20160525_1623.py
0005_auto_20170514_1141.py
0006_auto_20171214_2232.py

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Oct 25, 2023
Remove pinning for django-oauth-toolkit and remove
explicit declaration of oauthlib as it is a
dependency of django-oauth-toolkit.

Re-address prior work-around for older oauth2_provider
migration file silently failing to apply and holding up
all subsequent oauth2_provider migrations, as this migration
file, and a few subsequent ones, have now been squashed upstream.
"oauth2_provider" is part of django-oauth-toolkit.

Added dev logging for before/after our migration in this area.
phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Oct 27, 2023
As from Django Oauth Toolkit 2.x onwards, Oauth app
client_secret is hashed within dd, dictating that we can
no longer source this secret from the db for our internal
cli client app token requests. Move to establishing a dynamic
Oathapp client_secret, established in settings.py, and reset
by rockstor-bootstrap.service.

# Includes
- Adding a requests timeouts to client token requests.
- Arbitrary fsting application.
- Update disk, pool, share, snap state every 20s not every minute.
- Abandon bootstrap after 10, not 15 attempts.
@phillxnet
Copy link
Member Author

Note that this issue is also related to our recent move to Py3.9 as we have from:
https://django-oauth-toolkit.readthedocs.io/en/latest/changelog.html#id42

[1.4.0] 2021-02-08
... Added support for Python 3.9

@phillxnet
Copy link
Member Author

https://django-oauth-toolkit.readthedocs.io/en/latest/changelog.html#id49

[1.3.2] 2020-03-24
Fixed
Fixes: 1.3.1 inadvertently uploaded to pypi with an extra migration (0003…) from a dev branch.

phillxnet added a commit to phillxnet/rockstor-core that referenced this issue Oct 30, 2023
Remove pinning for django-oauth-toolkit and remove explicit
declaration of oauthlib as it is a dependency of django-oauth-toolkit.

Re-address prior work-around for older oauth2_provider
migration file silently failing to apply, and holding up
all subsequent oauth2_provider migrations, as this migration
file, and a few subsequent ones, have now been squashed upstream.
"oauth2_provider" is part of django-oauth-toolkit.

# Includes:
- Added logging for before & after django-oauth-toolkit migration.
- Adopt dynamic client_secret for internal Oauth app.
As from Django Oauth Toolkit 2.x onwards, Oauth app
client_secret is hashed within Django's database, dictating that
we can no longer source this secret from the db for our internal
cli client app token requests. Move to establishing a dynamic
Oathapp client_secret, established in settings.py, and reset by
rockstor-bootstrap.service, i.e. on each service restart/reboot.
- Adding a requests timeouts to client token requests.
- Arbitrary fsting application.
- Update disk, pool, share, snap state every 20s not every minute.
- Abandon rockstor-bootstrap.service start (boostrap scritp) after
10, not 15 attempts.
phillxnet added a commit that referenced this issue Nov 8, 2023
@phillxnet
Copy link
Member Author

Closing as:
Fixed by #2727

Which in turn depended on its spin-out/blocker of PR of #2733

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

No branches or pull requests

1 participant