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

Jormun: Feat avoid pg authentication for bad token #3893

Merged
merged 8 commits into from
Jan 17, 2023
Merged
Show file tree
Hide file tree
Changes from 6 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
11 changes: 6 additions & 5 deletions source/jormungandr/jormungandr/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,6 @@
from jormungandr.new_relic import record_custom_parameter
from jormungandr.authentication import get_user, get_token, get_app_name, get_used_coverages
from jormungandr._version import __version__
from jormungandr.utils import can_connect_to_database
import six


Expand Down Expand Up @@ -90,13 +89,15 @@ def add_info_newrelic(response, *args, **kwargs):
try:
record_custom_parameter('navitia-request-id', request.id)

if can_connect_to_database():
token = get_token()
token = get_token()
if token:
user = get_user(token=token, abort_if_no_token=False)
app_name = get_app_name(token)
if user:
record_custom_parameter('user_id', str(user.id))
record_custom_parameter('token_name', app_name)
# This method verifies database connection and gets object Key only once when cache expires.
app_name = get_app_name(token)
if app_name:
record_custom_parameter('token_name', app_name)

record_custom_parameter('version', __version__)
coverages = get_used_coverages()
Expand Down
11 changes: 7 additions & 4 deletions source/jormungandr/jormungandr/authentication.py
Original file line number Diff line number Diff line change
Expand Up @@ -107,7 +107,7 @@ def get_token():
# Python3 Compatibility 2: Decode bytes to string in order to use split()
if isinstance(decoded, bytes):
decoded = decoded.decode()
return decoded.split(':')[0]
return decoded.split(':')[0].strip()
except (binascii.Error, UnicodeDecodeError):
logging.getLogger(__name__).exception('badly formated token %s', auth)
flask_restful.abort(401, message="Unauthorized, invalid token", status=401)
Expand Down Expand Up @@ -141,6 +141,7 @@ def has_access(region, api, abort, user):
# if jormungandr is on public mode or database is not accessible, we skip the authentication process
logging.getLogger(__name__).debug('User "has_access" to region/api not cached')

# Connection to database verified only once when cache expires.
if current_app.config.get('PUBLIC', False) or (not can_connect_to_database()):
return True

Expand Down Expand Up @@ -193,11 +194,12 @@ def cache_get_user(token):

def uncached_get_user(token):
logging.getLogger(__name__).debug('Get User from token (uncached)')
if not can_connect_to_database():
logging.getLogger(__name__).debug('Cannot connect to database, we set User to None')
return None
try:
user = User.get_from_token(token, datetime.datetime.now())

# if user doesn't exist for a token, get default token with user_type = no_access
if not user:
user = User.get_without_access()
except Exception as e:
logging.getLogger(__name__).error('No access to table User (error: {})'.format(e))
g.can_connect_to_database = False
Expand All @@ -211,6 +213,7 @@ def uncached_get_user(token):
)
@cache.memoize(current_app.config[str('CACHE_CONFIGURATION')].get(str('TIMEOUT_AUTHENTICATION'), 300))
def cache_get_key(token):
# This verification is done only once when cache expires.
if not can_connect_to_database():
return None
try:
Expand Down
6 changes: 2 additions & 4 deletions source/jormungandr/jormungandr/instance_manager.py
Original file line number Diff line number Diff line change
Expand Up @@ -222,11 +222,9 @@ def stop(self):
def _filter_authorized_instances(self, instances, api):
if not instances:
return []
# During the period database is not accessible, all the instances are valid for the user.
if not can_connect_to_database():
return instances

# get_user is cached hence access to database only once when cache expires.
user = authentication.get_user(token=authentication.get_token())
# has_access returns true if can_connect_to_database = false when cache expires for each coverage
valid_instances = [
i for i in instances if authentication.has_access(i.name, abort=False, user=user, api=api)
]
Expand Down
7 changes: 6 additions & 1 deletion source/navitiacommon/navitiacommon/models/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ class User(db.Model, TimestampMixin): # type: ignore
)

type = db.Column(
db.Enum('with_free_instances', 'without_free_instances', 'super_user', name='user_type'),
db.Enum('with_free_instances', 'without_free_instances', 'super_user', 'no_access', name='user_type'),
default='with_free_instances',
nullable=False,
)
Expand Down Expand Up @@ -194,6 +194,11 @@ def get_from_key(cls, token, valid_until):
)
return query.first()

@classmethod
def get_without_access(cls):
query = cls.query.filter(cls.type == 'no_access')
return query.first()

def has_access(self, instance_id, api_name):
if self.is_super_user:
return True
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
"""Add a new type no_access in user_type

Revision ID: 04f9b89e3ef5
Revises: 75681b9c74aa
Create Date: 2022-12-30 11:23:58.436438

"""

# revision identifiers, used by Alembic.
revision = '04f9b89e3ef5'
down_revision = '75681b9c74aa'

from alembic import op
import sqlalchemy as sa

new_options = ('with_free_instances', 'without_free_instances', 'super_user', 'no_access')
old_type = sa.Enum('with_free_instances', 'without_free_instances', 'super_user', name='user_type')
new_type = sa.Enum(*new_options, name='user_type')
tmp_type = sa.Enum(*new_options, name='_user_type')


def upgrade():
tmp_type.create(op.get_bind(), checkfirst=False)
op.execute('ALTER TABLE "user" ALTER COLUMN type DROP DEFAULT')
op.execute('ALTER TABLE "user" ALTER COLUMN type TYPE _user_type USING type::text::_user_type')
old_type.drop(op.get_bind(), checkfirst=False)

new_type.create(op.get_bind(), checkfirst=False)
op.execute('ALTER TABLE "user" ALTER COLUMN type TYPE user_type USING type::text::user_type')
op.execute('ALTER TABLE "user" ALTER COLUMN type SET DEFAULT \'with_free_instances\'')
tmp_type.drop(op.get_bind(), checkfirst=False)
op.execute(
'INSERT INTO "user"(login, email, type) values(\'user_without_access\', \'[email protected]\', \'no_access\')'
)


def downgrade():
op.execute('DELETE FROM "user" where type = \'no_access\'')
tmp_type.create(op.get_bind(), checkfirst=False)
op.execute('ALTER TABLE "user" ALTER COLUMN type DROP DEFAULT')
op.execute('ALTER TABLE "user" ALTER COLUMN type TYPE _user_type USING type::text::_user_type')
new_type.drop(op.get_bind(), checkfirst=False)

old_type.create(op.get_bind(), checkfirst=False)
op.execute('ALTER TABLE "user" ALTER COLUMN type TYPE user_type USING type::text::user_type')
op.execute('ALTER TABLE "user" ALTER COLUMN type SET DEFAULT \'with_free_instances\'')
tmp_type.drop(op.get_bind(), checkfirst=False)
4 changes: 2 additions & 2 deletions source/tyr/tyr/resources.py
Original file line number Diff line number Diff line change
Expand Up @@ -1239,9 +1239,9 @@ def post(self):
type=str,
required=False,
default='with_free_instances',
help='type of user: [with_free_instances, without_free_instances, super_user]',
help='type of user: [with_free_instances, without_free_instances, super_user, no_access]',
location=('json', 'values'),
choices=['with_free_instances', 'without_free_instances', 'super_user'],
choices=['with_free_instances', 'without_free_instances', 'super_user', 'no_access'],
)
parser.add_argument('shape', type=geojson_argument, required=False, location=('json', 'values'))
parser.add_argument('default_coord', type=CoordFormat(), required=False, location=('json', 'values'))
Expand Down