-
Notifications
You must be signed in to change notification settings - Fork 28
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
Binding DB sessions based on SQLAlchemy 1, changing how to declare Base Model classes, and other code modernization #50
Changes from 5 commits
93b4de1
e063508
55c7d74
96aa71b
010a756
f4d595c
830c7a9
a509d64
a60326a
dbc3d62
5cc7e50
1b5c162
52771ce
f5afd8b
870463c
7e1db9f
0caca54
f24fc11
e6b31e0
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,12 +1,11 @@ | ||
import os | ||
import sys | ||
|
||
import cherrypy | ||
import settings | ||
from common import filters | ||
from common.utils import common_context, url_for | ||
from example import settings | ||
from jinja2 import Environment, FileSystemLoader | ||
from social_cherrypy.utils import backends, load_strategy | ||
from social_cherrypy.utils import load_strategy | ||
from social_cherrypy.views import CherryPyPSAViews | ||
from social_core.utils import setting_name | ||
|
||
|
@@ -15,7 +14,9 @@ | |
from .db.user import User | ||
|
||
BASE_DIR = os.path.dirname(os.path.dirname(os.path.abspath(__file__))) | ||
DATABASE_NAME = "sqlite:///{dbname}".format(dbname=os.path.join(BASE_DIR, "db.sqlite3")) | ||
DATABASE_NAME = "sqlite:///{dbname}".format( # fix: skip | ||
dbname=os.path.join(BASE_DIR, "db.sqlite3") | ||
) | ||
|
||
SAEnginePlugin(cherrypy.engine, DATABASE_NAME).subscribe() | ||
|
||
|
@@ -38,9 +39,9 @@ def render_home(self): | |
cherrypy.config[setting_name("AUTHENTICATION_BACKENDS")], | ||
load_strategy(), | ||
user=getattr(cherrypy.request, "user", None), | ||
plus_id=cherrypy.config.get(setting_name("SOCIAL_AUTH_GOOGLE_PLUS_KEY")), | ||
) | ||
return cherrypy.tools.jinja2env.get_template("home.html").render(**context) | ||
jinja2env = cherrypy.tools.jinja2env | ||
return jinja2env.get_template("home.html").render(**context) | ||
Comment on lines
+43
to
+44
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. The adjustments to template rendering calls improve clarity. However, ensure that HTML escaping is properly handled to prevent potential XSS vulnerabilities when using Jinja2 directly. |
||
|
||
|
||
def load_user(): | ||
|
@@ -56,22 +57,24 @@ def session_commit(): | |
|
||
|
||
def get_settings(module): | ||
not_in_items = ["__builtins__", "__file__"] | ||
return { | ||
key: value | ||
for key, value in module.__dict__.items() | ||
if key not in module.__builtins__ and key not in ["__builtins__", "__file__"] | ||
if key not in module.__builtins__ and key not in not_in_items | ||
} | ||
|
||
|
||
SOCIAL_SETTINGS = get_settings(settings) | ||
|
||
try: | ||
import local_settings | ||
from example import local_settings | ||
|
||
SOCIAL_SETTINGS.update(get_settings(local_settings)) | ||
except ImportError: | ||
raise RuntimeError( | ||
"Define a local_settings.py using " "local_settings.py.template as base" | ||
"Define a local_settings.py using " # fix: skip | ||
"local_settings.py.template as base" | ||
) | ||
|
||
|
||
|
@@ -82,7 +85,7 @@ def run_app(listen_address="0.0.0.0:8001"): | |
"server.socket_port": int(port), | ||
"server.socket_host": host, | ||
"tools.sessions.on": True, | ||
"tools.sessions.storage_type": "ram", | ||
"tools.sessions.storage_class": cherrypy.lib.sessions.RamSession, | ||
"tools.db.on": True, | ||
"tools.authenticate.on": True, | ||
"SOCIAL_AUTH_USER_MODEL": "example.db.user.User", | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,5 @@ | ||
from sqlalchemy.ext.declarative import declarative_base | ||
from sqlalchemy.orm import DeclarativeBase | ||
|
||
Base = declarative_base() | ||
|
||
class Base(DeclarativeBase): | ||
pass |
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -16,7 +16,8 @@ def associations(user, strategy): | |
return list(user_associations) | ||
|
||
|
||
def common_context(authentication_backends, strategy, user=None, plus_id=None, **extra): | ||
def common_context( # fix: skip | ||
authentication_backends, strategy, user=None, plus_id=None, **extra): | ||
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. The use of |
||
"""Common view context""" | ||
context = { | ||
"user": user, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
from django.contrib import admin | ||
from django.contrib import admin # noqa: F401 | ||
|
||
# Register your models here. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
from django.test import TestCase | ||
from django.test import TestCase # noqa: F401 | ||
|
||
# Create your tests here. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,12 @@ | ||
import json | ||
|
||
from django.conf import settings | ||
from django.contrib.auth import login | ||
from django.contrib.auth import logout as auth_logout | ||
from django.contrib.auth.decorators import login_required | ||
from django.http import HttpResponse, HttpResponseBadRequest | ||
from django.shortcuts import redirect | ||
from social_core.backends.google import GooglePlusAuth | ||
from social_core.backends.oauth import BaseOAuth1, BaseOAuth2 | ||
from social_core.backends.utils import load_backends | ||
from social_django.utils import load_strategy, psa | ||
from social_django.utils import psa | ||
|
||
from .decorators import render_to | ||
|
||
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.
Detected a potential XSS vulnerability in the |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
from django.contrib import admin | ||
from django.contrib import admin # noqa: F401 | ||
|
||
# Register your models here. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,3 +1,3 @@ | ||
from django.test import TestCase | ||
from django.test import TestCase # noqa: F401 | ||
|
||
# Create your tests here. |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,15 +1,12 @@ | ||
import json | ||
|
||
from django.conf import settings | ||
from django.contrib.auth import login | ||
from django.contrib.auth import logout as auth_logout | ||
from django.contrib.auth.decorators import login_required | ||
from django.http import HttpResponse, HttpResponseBadRequest | ||
from django.shortcuts import redirect | ||
from social_core.backends.google import GooglePlusAuth | ||
from social_core.backends.oauth import BaseOAuth1, BaseOAuth2 | ||
from social_core.backends.utils import load_backends | ||
from social_django.utils import load_strategy, psa | ||
from social_django.utils import psa | ||
|
||
from .decorators import render_to | ||
|
||
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.
Detected direct rendering of data to the end user via 'HttpResponse'. This could potentially lead to Cross-Site Scripting (XSS) vulnerabilities if the data is not properly sanitized. Consider using Django's template engine to safely render HTML, which automatically escapes variables unless explicitly marked otherwise. - return HttpResponse(json.dumps(data), mimetype="application/json")
+ return JsonResponse(data) Note: |
||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
from example.models import user | ||
from social_flask_sqlalchemy import models | ||
from example.models import user # noqa: F401 | ||
from social_flask_sqlalchemy import models # noqa: F401 |
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -1,2 +1,2 @@ | ||
from example.routes import main | ||
from social_flask import routes | ||
from example.routes import main # noqa: F401 | ||
from social_flask import routes # noqa: F401 |
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.
The update to the database connection string formatting using
format
is correct and improves readability. However, consider using f-strings for string formatting as they are more readable and efficient in Python 3.6+.Committable suggestion