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

Implementation of Oauth of Github, Google and Microsoft #4298

Open
wants to merge 95 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 19 commits
Commits
Show all changes
95 commits
Select commit Hold shift + click to select a range
63d5f9b
not working api
feyruzb May 17, 2024
3d6ced1
Backup before OAuth integration changes
feyruzb Jul 3, 2024
74c1202
commit with new version of code ,work in progress
feyruzb Jul 4, 2024
3c0b13f
second backup
feyruzb Jul 17, 2024
d75edbd
github working implementation
feyruzb Jul 18, 2024
47ba1dd
working github implementation , allows everyone to log in
feyruzb Jul 18, 2024
822c626
working impelementation of github Oauth
feyruzb Jul 18, 2024
251f7dd
removed useless server configuration variables, useless debug message…
feyruzb Jul 19, 2024
1394f28
added dynamic path for finding server_config.json
feyruzb Jul 19, 2024
785a0f0
refactored authentication process
feyruzb Jul 22, 2024
be35303
refactored code for config oauth reusability ,improved performace
feyruzb Jul 22, 2024
370908e
updated server config template
feyruzb Jul 22, 2024
9bc3482
added google authentification
feyruzb Jul 24, 2024
422aacb
changed the process of finding which login provider to use , now it s…
feyruzb Jul 25, 2024
feabac5
added documentation for oauth
feyruzb Jul 29, 2024
1967161
fixed a part of mistakes
feyruzb Jul 30, 2024
0b5f571
added mock server, test case
feyruzb Aug 8, 2024
2faf4f2
added test cases and generalized functions, added dynamic appearence …
feyruzb Aug 9, 2024
a9713c9
github is now using email as username
feyruzb Aug 12, 2024
bdbd6b6
fixed cservakt suggestions
feyruzb Sep 3, 2024
1b6c87a
Update docs/web/authentication.md
feyruzb Sep 4, 2024
4ab45de
documentation changed
feyruzb Sep 4, 2024
472efad
Merge branch 'branch-2-backup' of https://github.com/feyruzb/codechec…
feyruzb Sep 4, 2024
3c78f2d
removed unnecessary "oauth" prefix, now provider in auth_string
feyruzb Sep 4, 2024
3e32aa7
working implementation of microsoft OAuth
feyruzb Sep 10, 2024
066ef7b
added FIX ME near workaround
feyruzb Sep 11, 2024
3c83d1e
added FIX ME near workaround
feyruzb Sep 11, 2024
77f6fc5
added state check against stolen session
feyruzb Sep 18, 2024
d5c9a8c
fixed half the problems from review
feyruzb Sep 20, 2024
fc930b3
changed syntaxis to much the rest of the file, added a check for defa…
feyruzb Sep 23, 2024
ff7c83f
not working api
feyruzb May 17, 2024
8ef45dd
Backup before OAuth integration changes
feyruzb Jul 3, 2024
0f02c88
commit with new version of code ,work in progress
feyruzb Jul 4, 2024
d50718d
second backup
feyruzb Jul 17, 2024
2050b3c
github working implementation
feyruzb Jul 18, 2024
cd7d61f
working github implementation , allows everyone to log in
feyruzb Jul 18, 2024
7095028
working impelementation of github Oauth
feyruzb Jul 18, 2024
e72c224
removed useless server configuration variables, useless debug message…
feyruzb Jul 19, 2024
b501310
added dynamic path for finding server_config.json
feyruzb Jul 19, 2024
0066143
refactored authentication process
feyruzb Jul 22, 2024
99f07b5
refactored code for config oauth reusability ,improved performace
feyruzb Jul 22, 2024
2ffe925
updated server config template
feyruzb Jul 22, 2024
1f0692d
added google authentification
feyruzb Jul 24, 2024
85f7d63
changed the process of finding which login provider to use , now it s…
feyruzb Jul 25, 2024
95288d3
added documentation for oauth
feyruzb Jul 29, 2024
dd77fbb
fixed a part of mistakes
feyruzb Jul 30, 2024
e1d47e1
added mock server, test case
feyruzb Aug 8, 2024
c286c7c
added test cases and generalized functions, added dynamic appearence …
feyruzb Aug 9, 2024
9ba1d12
github is now using email as username
feyruzb Aug 12, 2024
ec3be0a
fixed cservakt suggestions
feyruzb Sep 3, 2024
993cf33
documentation changed
feyruzb Sep 4, 2024
87731a4
Update docs/web/authentication.md
feyruzb Sep 4, 2024
05e58db
removed unnecessary "oauth" prefix, now provider in auth_string
feyruzb Sep 4, 2024
40f8a07
working implementation of microsoft OAuth
feyruzb Sep 10, 2024
71e35c5
added FIX ME near workaround
feyruzb Sep 11, 2024
09f49c4
added FIX ME near workaround
feyruzb Sep 11, 2024
a8a1503
added state check against stolen session
feyruzb Sep 18, 2024
0b2d7b3
fixed half the problems from review
feyruzb Sep 20, 2024
c8e0c3a
changed syntaxis to much the rest of the file, added a check for defa…
feyruzb Sep 23, 2024
ebd1aa3
Merge branch 'branch-2-backup' of https://github.com/feyruzb/codechec…
feyruzb Sep 24, 2024
9595370
Merge branch 'Ericsson:master' into branch-2-backup
feyruzb Sep 24, 2024
5256b0e
Merge branch 'branch-2-backup' of https://github.com/feyruzb/codechec…
feyruzb Sep 24, 2024
e37683a
tests check
feyruzb Sep 24, 2024
4a5c76b
fixed bug of invalid authentication
feyruzb Sep 24, 2024
93e0605
fixed Username:Password vulnerability of accepting illegal OAuth format
feyruzb Sep 25, 2024
afb9323
added try catches for AUTHLIB functions
feyruzb Sep 25, 2024
c99d550
log in different devices with the same username in the same session
feyruzb Sep 26, 2024
cc9ce96
removed session reuse, added server side state check
feyruzb Oct 1, 2024
6a44b25
testing test cases
feyruzb Oct 3, 2024
c96c10d
Merge branch 'Ericsson:master' into branch-2-backup
feyruzb Oct 7, 2024
b71af06
test of new table in database and check try
feyruzb Oct 14, 2024
152f0d9
working implementation of state check
feyruzb Oct 15, 2024
d6561b8
aded new column in auth_sessions for storing access_token for later v…
feyruzb Oct 15, 2024
21d3a8a
Merge branch 'Ericsson:master' into branch-2-backup
feyruzb Oct 16, 2024
65adbbd
pkce problem
feyruzb Oct 22, 2024
1617633
pkce implemented
feyruzb Oct 24, 2024
3d22dfb
Merge pull request #1 from feyruzb/pkce_problem
feyruzb Oct 24, 2024
0ac3d52
Merge branch 'Ericsson:master' into branch-2-backup
feyruzb Oct 29, 2024
c396d59
Merge branch 'Ericsson:master' into branch-2-backup
feyruzb Nov 5, 2024
0caea7e
fixed npm warnings
feyruzb Nov 5, 2024
39a3ad4
protection against expired session use
feyruzb Nov 6, 2024
161106c
Merge branch 'Ericsson:master' into branch-2-backup
feyruzb Nov 16, 2024
3f774da
mix-up attack protection
feyruzb Nov 22, 2024
8cd4793
Merge branch 'Ericsson:master' into branch-2-backup
feyruzb Nov 27, 2024
ad5beec
fixed unnescessary passing of state and code_challenge to client
feyruzb Dec 9, 2024
e3d6d7a
removed oauth_data_id, was prone to attack, now uses state for queryi…
feyruzb Dec 9, 2024
6148c4d
Add check for invalid callback URL format and enhance documentation
feyruzb Jan 7, 2025
4f083b4
Merge remote-tracking branch 'upstream/master' into branch-2-backup
feyruzb Jan 7, 2025
1df88ff
testing test cases
feyruzb Jan 9, 2025
0b9f8f9
working implementation of callback_url format check
feyruzb Jan 9, 2025
15d710f
removed unnescessary try catch
feyruzb Jan 9, 2025
2aadc99
fixed documentation inconsistencies and added github pkce problem des…
feyruzb Jan 9, 2025
4393e55
added documentation that explains the resoning behind fetching primar…
feyruzb Jan 9, 2025
4b6cc2e
modified documentation and format checker for callback url
feyruzb Jan 10, 2025
e18d6ab
now during the login process all providers send refresh token during …
feyruzb Jan 15, 2025
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
98 changes: 98 additions & 0 deletions docs/web/authentication.md
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,9 @@ Table of Contents
* [<i>LDAP</i> authentication](#ldap-authentication)
* [Configuration options](#configuration-options)
* Membership in custom groups with [<i>regex_groups</i>](#regex_groups-authentication)
* [<i>OAUTH</i> authentication](#oauth-authentication)
feyruzb marked this conversation as resolved.
Show resolved Hide resolved
* [<i>OAUTH</i> Configuration options](#oauth-configuration-options)
* [<i>OAUTH</i> details per each provider](#oauth-details-per-each-provider)
* [Client-side configuration](#client-side-configuration)
* [Web-browser client](#web-browser-client)
* [Command-line client](#command-line-client)
Expand Down Expand Up @@ -320,6 +323,101 @@ groups. For more information [see](permissions.md#managing-permissions).

----

### <i>OAUTH</i> authentication <a name="oauth-authentication"></a>

CodeChecker also supports OAUTH-based authentication. The `authentication.method_oauth` section contains the configuration for OAUTH authentication for different OAUTH providers. The server can be configured for different Oauth `providers` .Users can be added into the `allowed_users`
feyruzb marked this conversation as resolved.
Show resolved Hide resolved

#### OAUTH Configuration options <a name="oauth-configuration-options"></a>
* `enabled`

Indicated if OAUTH authentication is enabled (required for any methods below)
Copy link
Contributor

Choose a reason for hiding this comment

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

Official capitalization is OAuth.

Copy link
Collaborator Author

@feyruzb feyruzb Jan 9, 2025

Choose a reason for hiding this comment

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

fixed, renamed


* `providers`

The provider field contains configuration details for OAuth providers. Each provider's configuration includes but may vary depending on provider:
feyruzb marked this conversation as resolved.
Show resolved Hide resolved

* `enabled`

Indicates if current provider is enabled (github, google, etc)

* `oauth_client_id`

Contains client ID provided by the OAuth provider.


* `oauth_client_secret`

The client secret must be provided by the OAuth provider.

* `oauth_authorization_uri`

This link in used for redirecting user for provider's authentication page

* `oauth_redirect_uri`

User will be redirected back to the provided link after login with returned data.

* `oauth_token_uri`

The URI to exchange the authorization code for an access token.

* `oauth_user_info_uri`

The URI to fetch the authenticated user's information.

* `oauth_scope`

The scope of access requested from the OAuth provider.

* `oauth_user_info_mapping`

A mapping of user info fields from the provider to local fields.

* `username`

Field for the username.
* `email`

Field for the email.
* `fullname`

Field for the fullname.
* `allowed_users`

A list of allowed users differently configured for each provider
feyruzb marked this conversation as resolved.
Show resolved Hide resolved

~~~{.json}
"method_oauth": {
"enabled": false,
"providers": {
"example_provider": {
"enabled": false,
"oauth_client_id": "client id",
"oauth_client_secret": "client secret",
"oauth_authorization_uri": "https://accounts.google.com/o/oauth2/auth",
"oauth_redirect_uri": "http://localhost:8080/login",
"oauth_token_uri": "https://accounts.google.com/o/oauth2/token",
"oauth_user_info_uri": "https://www.googleapis.com/oauth2/v1/userinfo",
"oauth_scope": "openid email profile",
"oauth_user_info_mapping": {
"username": "email",
"email": "email",
"fullname": "name"
},
"allowed_users": [
"user1",
"user2",
"user3"
]
}
}
}
~~~

#### Oauth Details per each provider <a name ="oauth-details-per-each-provider"></a>

* For OAuth providers to function correctly, the `oauth_redirect_uri` in application's configuration must exactly match the `Authorized redirect URIs` specified in the Google API Console.
feyruzb marked this conversation as resolved.
Show resolved Hide resolved

# Client-side configuration <a name="client-side-configuration"></a>

## Web-browser client <a name="web-browser-client"></a>
Expand Down
7 changes: 7 additions & 0 deletions web/api/authentication.thrift
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,13 @@ service codeCheckerAuthentication {
string performLogin(1: string authMethod,
2: string authString)
throws (1: codechecker_api_shared.RequestFailed requestError),

// Returns list of providers for oauth for respective appearence of buttons.
list<string> getOauthProviders(),

// Create a link for the user to log in for github Oauth.
string createLink(1: string provider)
throws (1: codechecker_api_shared.RequestFailed requestError),

// Performs logout action for the user. Must be called from the
// corresponding valid session which is to be destroyed.
Expand Down
Binary file not shown.
Binary file not shown.
2 changes: 1 addition & 1 deletion web/api/js/codechecker-api-node/package.json
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
{
"name": "codechecker-api",
"version": "6.58.0",
"version": "6.59.0",
"description": "Generated node.js compatible API stubs for CodeChecker server.",
"main": "lib",
"homepage": "https://github.com/Ericsson/codechecker",
Expand Down
Binary file modified web/api/py/codechecker_api/dist/codechecker_api.tar.gz
Binary file not shown.
2 changes: 1 addition & 1 deletion web/api/py/codechecker_api/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
with open('README.md', encoding='utf-8', errors="ignore") as f:
long_description = f.read()

api_version = '6.58.0'
api_version = '6.59.0'

setup(
name='codechecker_api',
Expand Down
Binary file not shown.
2 changes: 1 addition & 1 deletion web/api/py/codechecker_api_shared/setup.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,7 @@
with open('README.md', encoding='utf-8', errors="ignore") as f:
long_description = f.read()

api_version = '6.58.0'
api_version = '6.59.0'

setup(
name='codechecker_api_shared',
Expand Down
2 changes: 1 addition & 1 deletion web/client/codechecker_client/client.py
Original file line number Diff line number Diff line change
Expand Up @@ -12,9 +12,9 @@

import getpass
import sys

feyruzb marked this conversation as resolved.
Show resolved Hide resolved
from thrift.Thrift import TApplicationException


import codechecker_api_shared
from codechecker_api.Authentication_v6 import ttypes as AuthTypes

Expand Down
4 changes: 4 additions & 0 deletions web/client/codechecker_client/helpers/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,10 @@ def getAccessControl(self):
def performLogin(self, auth_method, auth_string):
pass

@thrift_client_call
def createLink(self, provider):
feyruzb marked this conversation as resolved.
Show resolved Hide resolved
pass

@thrift_client_call
def destroySession(self):
pass
Expand Down
2 changes: 1 addition & 1 deletion web/codechecker_web/shared/version.py
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,7 @@
# The newest supported minor version (value) for each supported major version
# (key) in this particular build.
SUPPORTED_VERSIONS = {
6: 58
6: 59
}

# Used by the client to automatically identify the latest major and minor
Expand Down
2 changes: 2 additions & 0 deletions web/requirements.txt
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@ gitpython==3.1.41
PyYAML==6.0.1
types-PyYAML==6.0.12.12
sarif-tools==1.0.0
Authlib==1.3.1
requests==2.32.3
feyruzb marked this conversation as resolved.
Show resolved Hide resolved

./api/py/codechecker_api/dist/codechecker_api.tar.gz
./api/py/codechecker_api_shared/dist/codechecker_api_shared.tar.gz
90 changes: 89 additions & 1 deletion web/server/codechecker_server/api/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,8 @@
Handle Thrift requests for authentication.
"""

from authlib.integrations.requests_client import OAuth2Session
from authlib.common.security import generate_token

import json
import codechecker_api_shared
Expand All @@ -30,6 +32,7 @@
from ..server import permissions
from ..session_manager import generate_session_token


LOG = get_logger('server')


Expand Down Expand Up @@ -92,9 +95,39 @@ def getAuthParameters(self):
def getLoggedInUser(self):
return self.__auth_session.user if self.__auth_session else ""

@timeit
def createLink(self, provider):
"""
For creating a autehntication link for OAuth for specified provider
"""
oauth_config = self.__manager.get_oauth_config(provider)
if not oauth_config.get("enabled"):
raise codechecker_api_shared.ttypes.RequestFailed(
codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED,
"OAuth authentication is not enabled.")

client_id = oauth_config["oauth_client_id"]
client_secret = oauth_config["oauth_client_secret"]
scope = oauth_config["oauth_scope"]
authorization_uri = oauth_config["oauth_authorization_uri"]
redirect_uri = oauth_config["oauth_redirect_uri"]

# Create an OAuth2Session instance
session = OAuth2Session(
Copy link
Contributor

Choose a reason for hiding this comment

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

Should this be reused for the current authentication method? Currently a new session is made for every login attempt.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

I made it so every new login attempts, deletes all sessions with that username, is that a fine solution?

Copy link
Contributor

Choose a reason for hiding this comment

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

That's not really what I meant here - what I meant was reusing this OAuth2Session object, as it keeps the internal state which helps verify the authenticity of the login attempts. If this is not feasible, consider setting session.state and other variables manually into the newly created object here. I don't think there's a need to invalidate other logins every time there is one - this is not the case for other login methods either.

client_id,
client_secret,
scope=scope,
redirect_uri=redirect_uri)

# Create authorization URL
nonce = generate_token()
url = session.create_authorization_url(
authorization_uri, nonce=nonce)[0]
return url

@timeit
def getAcceptedAuthMethods(self):
return ["Username:Password"]
return ["Username:Password", "oauth"]

@timeit
def getAccessControl(self):
Expand Down Expand Up @@ -142,6 +175,10 @@ def getAccessControl(self):
globalPermissions=global_permissions,
productPermissions=product_permissions)

@timeit
def getOauthProviders(self):
return self.__manager.get_oauth_providers()

@timeit
def performLogin(self, auth_method, auth_string):
if not auth_string:
Expand All @@ -167,6 +204,57 @@ def performLogin(self, auth_method, auth_string):
codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED,
msg)

elif auth_method.startswith("oauth_", 0, 6):
feyruzb marked this conversation as resolved.
Show resolved Hide resolved
provider = auth_method[6:]
oauth_config = self.__manager.get_oauth_config(provider)
if not oauth_config.get("enabled"):
raise codechecker_api_shared.ttypes.RequestFailed(
codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED,
"OAuth authentication is not enabled.")

client_id = oauth_config["oauth_client_id"]
client_secret = oauth_config["oauth_client_secret"]
scope = oauth_config["oauth_scope"]
token_url = oauth_config["oauth_token_uri"]
user_info_url = oauth_config["oauth_user_info_uri"]
redirect_uri = oauth_config["oauth_redirect_uri"]
allowed_users = oauth_config.get("allowed_users", [])

session = OAuth2Session(
client_id,
client_secret,
scope=scope,
redirect_uri=redirect_uri)
token = session.fetch_token(
Copy link
Contributor

Choose a reason for hiding this comment

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

Exceptions from fetch_token and other OAuth methods should be caught and rethrown.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed

url=token_url,
authorization_response=auth_string)

user_info = session.get(user_info_url).json()
username = user_info[
oauth_config["oauth_user_info_mapping"]["username"]]

if provider == "github" and \
Copy link
Contributor

Choose a reason for hiding this comment

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

Are providers case-sensitive?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

yes

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

fixed, now whatever the performLogin() gets as provider is being transformed into lowercase

"localhost" not in user_info_url:
link = "https://api.github.com/user/emails"
for email in session.get(link).json():
if email['primary']:
username = email['email']
break

if allowed_users == ["*"] or username in allowed_users:
session = self.__manager.create_session(
provider + "@" + username +
":" + token['access_token'])
feyruzb marked this conversation as resolved.
Show resolved Hide resolved
return session.token
Copy link
Contributor

Choose a reason for hiding this comment

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

What happens when an OAuth user changes their password on the OAuth service? Or their account gets flagged for fraud or similar? According to the OAuth service their account will be disabled and the access_token revoked, but on the CodeChecker service they will stay logged in.
The session manager must store the access_token associated with the session, and periodically check the validity of the stored access_token on user interaction. It must also handle refreshing it using the refresh_token, if the access_token expired. (These could both be handled by authlib.)


if len(allowed_users) == 0:
raise codechecker_api_shared.ttypes.RequestFailed(
codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED,
"The allowed users list is empty")
feyruzb marked this conversation as resolved.
Show resolved Hide resolved

raise codechecker_api_shared.ttypes.RequestFailed(
codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED,
"User is not authorized to access this service")
raise codechecker_api_shared.ttypes.RequestFailed(
codechecker_api_shared.ttypes.ErrorCode.AUTH_DENIED,
"Could not negotiate via common authentication method.")
Expand Down
Loading
Loading