Skip to content

Commit

Permalink
Merge pull request #3893 from hove-io/feat_avoid_pg_authentification_…
Browse files Browse the repository at this point in the history
…for_bad_token

Feat avoid pg authentification for bad token
  • Loading branch information
Krishna Prasad ADHIKARI authored Jan 17, 2023
2 parents 7b9fc70 + 22ad94b commit 5abdae1
Show file tree
Hide file tree
Showing 5 changed files with 69 additions and 19 deletions.
23 changes: 13 additions & 10 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,18 +89,22 @@ 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()
# No log will be added in newrelic if token is absent in the request
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)

record_custom_parameter('version', __version__)
coverages = get_used_coverages()
if coverages:
record_custom_parameter('coverage', coverages[0])
# 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__)
# No access to database required
coverages = get_used_coverages()
if coverages:
record_custom_parameter('coverage', coverages[0])
except:
logger = logging.getLogger(__name__)
logger.exception('error while reporting to newrelic:')
Expand Down
12 changes: 8 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,13 @@ 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()
logging.getLogger(__name__).warning('Invalid token : {}'.format(token[0:10]))
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 +214,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' and cls.login == 'user_without_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,40 @@
"""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():
op.execute("COMMIT")
op.execute("ALTER TYPE user_type ADD VALUE 'no_access'")
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)

0 comments on commit 5abdae1

Please sign in to comment.