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

Feat: Access logs #198

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open

Feat: Access logs #198

wants to merge 9 commits into from

Conversation

deepaksuresh2411
Copy link
Contributor

No description provided.

@deepaksuresh2411 deepaksuresh2411 changed the title wip: capture access logs Feat: Access logs Apr 11, 2024

class AccesslogsAppConfig(AppConfig):

name = "zelthy.apps.accesslogs"
Copy link
Contributor

Choose a reason for hiding this comment

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

let's use the app name as access_logs

from django.db import models
from axes.models import AccessBase

from ..appauth.models import AppUserModel, UserRoleModel
Copy link
Contributor

Choose a reason for hiding this comment

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

can we use the full import here ?
from zelthy.apps.appauth.models import AppUserModel, UserRoleModel

if anything needs to import within the app then it's still fine, but across the app we can use the full import
Let me know your thoughts

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah, we can follow that

from ..appauth.models import AppUserModel, UserRoleModel


class AppAccessLogs(AccessBase):
Copy link
Contributor

Choose a reason for hiding this comment

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

we can name this model as AppAccessLog

id=getattr(request, "selected_role_id")
).last()

elif getattr(request, "parser_context", ""):
Copy link
Contributor

Choose a reason for hiding this comment

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

is role_name getting set anywhere in parser_context ?

<div class="error-content">
<div class="not-found">Account Locked</div>
<div class="not-found-title">
Oops, your account/ IP address has been temporarily locked as there has been too many incorrect password attempts. Don't worry, this will be automatically reset after 15 minutes.
Copy link
Contributor

Choose a reason for hiding this comment

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

Can we have the cool off time dynamic here instead of hardcoded 15 mins ?

@@ -0,0 +1,48 @@
from django.contrib.auth import signals
Copy link
Contributor

Choose a reason for hiding this comment

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

reorder the import statements, pls check in all the files

from ..appauth.models import AppUserModel


def capture_failed_login_attempt(request, credentials):
Copy link
Contributor

Choose a reason for hiding this comment

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

add doc string in this method, as end user might be using it.



# Axes Lockout
env = environ.Env(
Copy link
Contributor

Choose a reason for hiding this comment

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

this initialization is not part of core settings, it's in the project template settings file zelthy3/backend/src/zelthy/cli/project_template/project_name/settings.py

Copy link
Contributor

Choose a reason for hiding this comment

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

in the core settings the default value can be present, and in the project settings we will have values which can be read from env

@@ -0,0 +1,40 @@
{% load i18n %}
Copy link
Contributor

Choose a reason for hiding this comment

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

This is a base template and this can be used for other error pages as well, should we move this to some other location let's say core/templates/exceptions ?



def user_authentication_failed(request, credentials):

Copy link
Contributor

Choose a reason for hiding this comment

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

add doc string in this method, check in all the methods

capture_failed_login_attempt(request, credentials)

if not AxesProxyHandler.is_allowed(request):
return {
Copy link
Contributor

Choose a reason for hiding this comment

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

I think we should use get_api_response method here, to maintain the consistent format of API response

@@ -116,3 +116,15 @@
STATIC_ROOT = os.path.join(BASE_DIR, "static")
STATIC_URL = "static/"
STATICFILES_DIRS += [os.path.join(BASE_DIR, "assets")]

# Axes Lockout
env = environ.Env(
Copy link
Contributor

Choose a reason for hiding this comment

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

No need to initialize it over here, it's initialized above (line no 9)
you can just update your default values in it

@@ -1,6 +1,7 @@
import sys
import os

import environ
Copy link
Contributor

Choose a reason for hiding this comment

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

unused import

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

Successfully merging this pull request may close these issues.

None yet

2 participants