forked from openedx/edx-enterprise
-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat: adds support for OAuth 2 client credentials auth in xapi #9
Open
tecoholic
wants to merge
6
commits into
opencraft-release/nutmeg.2
Choose a base branch
from
tecoholic/BB-7245-add-oauth2-client-cred-support-to-xapi
base: opencraft-release/nutmeg.2
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
Show all changes
6 commits
Select commit
Hold shift + click to select a range
87494c8
feat: adds support for OAuth 2 client credentials auth in xapi
tecoholic 29fbf5e
fix: consider all 2xx HTTP codes as successful xapi statement
tecoholic b7775b4
fix: quality issues
tecoholic c9872ee
feat: adds ability to send plain course, courserun ids in xapi
tecoholic bc64c2c
fix: failing tests in the xapi module
tecoholic abfed95
fix: quality issues
tecoholic File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
28 changes: 28 additions & 0 deletions
28
integrated_channels/xapi/migrations/0008_adds_auth_method_scope_and_url.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,28 @@ | ||
# Generated by Django 3.2.12 on 2023-09-01 07:26 | ||
|
||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('xapi', '0007_auto_20220405_2311'), | ||
] | ||
|
||
operations = [ | ||
migrations.AddField( | ||
model_name='xapilrsconfiguration', | ||
name='auth_method', | ||
field=models.CharField(choices=[('BASIC', 'HTTP Basic'), ('OAUTH2_CC', 'OAuth 2.0 Client Credentials')], default='BASIC', help_text='The Authentication Method to use when sending the xAPI data to the endpoint.', max_length=16, verbose_name='xAPI POST Authentication Method'), | ||
), | ||
migrations.AddField( | ||
model_name='xapilrsconfiguration', | ||
name='auth_url', | ||
field=models.URLField(blank=True, help_text='URL to use for authentication. Eg., Token URL for OAuth', null=True), | ||
), | ||
migrations.AddField( | ||
model_name='xapilrsconfiguration', | ||
name='oauth_scope', | ||
field=models.CharField(blank=True, help_text='The "scope" to pass for OAuth authentication.', max_length=255, null=True, verbose_name='OAuth scope'), | ||
), | ||
] |
18 changes: 18 additions & 0 deletions
18
...d_channels/xapi/migrations/0009_adds_plain_course_id_in_statements_flag_to_xapi_config.py
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,18 @@ | ||
# Generated by Django 3.2.12 on 2023-09-13 05:33 | ||
|
||
from django.db import migrations, models | ||
|
||
|
||
class Migration(migrations.Migration): | ||
|
||
dependencies = [ | ||
('xapi', '0008_adds_auth_method_scope_and_url'), | ||
] | ||
|
||
operations = [ | ||
migrations.AddField( | ||
model_name='xapilrsconfiguration', | ||
name='plain_course_id_in_statements', | ||
field=models.BooleanField(default=False, help_text='Uses the plain course ID (eg., course-v1:X+Y+Z) instead of the URI format in xAPI statements.', verbose_name='Use plain Course ID in xAPI Statements'), | ||
), | ||
] |
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -3,8 +3,12 @@ | |
""" | ||
|
||
import base64 | ||
from functools import cached_property | ||
|
||
import requests | ||
|
||
from django.contrib import auth | ||
from django.core.exceptions import ValidationError | ||
from django.db import models | ||
from django.utils.translation import gettext_lazy as _ | ||
|
||
|
@@ -16,6 +20,11 @@ | |
User = auth.get_user_model() | ||
|
||
|
||
class XAPIAuthMethods(models.TextChoices): | ||
HTTP_BASIC = 'BASIC', _('HTTP Basic') | ||
OAUTH2_CLIENT_CREDS = 'OAUTH2_CC', _('OAuth 2.0 Client Credentials') | ||
|
||
|
||
class XAPILRSConfiguration(TimeStampedModel): | ||
""" | ||
xAPI LRS configurations. | ||
|
@@ -39,6 +48,32 @@ class XAPILRSConfiguration(TimeStampedModel): | |
null=False, | ||
help_text=_('Is this configuration active?'), | ||
) | ||
auth_method = models.CharField( | ||
max_length=16, | ||
verbose_name="xAPI POST Authentication Method", | ||
choices=XAPIAuthMethods.choices, | ||
default=XAPIAuthMethods.HTTP_BASIC, | ||
help_text=_('The Authentication Method to use when sending the xAPI data to the endpoint.') | ||
) | ||
auth_url = models.URLField( | ||
blank=True, | ||
null=True, | ||
help_text=_("URL to use for authentication. Eg., Token URL for OAuth") | ||
) | ||
oauth_scope = models.CharField( | ||
max_length=255, | ||
verbose_name=_('OAuth scope'), | ||
blank=True, | ||
null=True, | ||
help_text=_('The "scope" to pass for OAuth authentication.') | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. What scopes are available here? Could we list them? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This would be specified by the LRS as well. |
||
) | ||
plain_course_id_in_statements = models.BooleanField( | ||
default=False, | ||
blank=False, | ||
null=False, | ||
verbose_name=_("Use plain Course ID in xAPI Statements"), | ||
help_text=_('Uses the plain course ID (eg., course-v1:X+Y+Z) instead of the URI format in xAPI statements.') | ||
) | ||
|
||
class Meta: | ||
app_label = 'xapi' | ||
|
@@ -62,10 +97,43 @@ def authorization_header(self): | |
""" | ||
Authorization header for authenticating requests to LRS. | ||
""" | ||
if self.auth_method == XAPIAuthMethods.OAUTH2_CLIENT_CREDS: | ||
return f'Bearer {self.access_token}' | ||
|
||
return 'Basic {}'.format( | ||
base64.b64encode('{key}:{secret}'.format(key=self.key, secret=self.secret).encode()).decode() | ||
) | ||
|
||
def clean(self): | ||
errors = {} | ||
# Don't allow OAuth2 Client Credentials method to be set without auth_url, or oauth_scope | ||
if self.auth_method == XAPIAuthMethods.OAUTH2_CLIENT_CREDS: | ||
if not self.auth_url: | ||
errors['auth_url'] = _("Authentication URL is required for the OAuth2 authentication method.") | ||
if not self.oauth_scope: | ||
errors['oauth_scope'] = _("OAuth scope is required for the OAuth2 authentication method.") | ||
|
||
if errors: | ||
raise ValidationError(errors) | ||
|
||
@cached_property | ||
def access_token(self): | ||
""" | ||
Gets the access token from the OAuth2 Authentication endpoint return it. | ||
""" | ||
if self.auth_method != XAPIAuthMethods.OAUTH2_CLIENT_CREDS: | ||
raise RuntimeError("Access Token can be fetched only for OAuth2 Client Credentials authenication method.") | ||
|
||
data = { | ||
"grant_type": "client_credentials", | ||
"scope": self.oauth_scope, | ||
"client_id": self.key, | ||
"client_secret": self.secret | ||
} | ||
response = requests.post(self.auth_url, data=data) | ||
response.raise_for_status() | ||
return response.json()["access_token"] | ||
|
||
|
||
class XAPILearnerDataTransmissionAudit(LearnerDataTransmissionAudit): | ||
""" | ||
|
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this point to the LMS OAuth2 (e.g.,
localhost:18000/oauth2/access_token
) or directly to theedx-enterprise
service? It would be good to clarify this here.There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ah. I think I have badly explained this PR. The OAuth URL would be from an external LRS service. For example, edCast uses
<instance-hostname>/api/lrs/v1/xapi/oauth2/token
.