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

Binding DB sessions based on SQLAlchemy 1, changing how to declare Base Model classes, and other code modernization #50

Open
wants to merge 19 commits into
base: master
Choose a base branch
from
Open
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
15 changes: 15 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,18 @@ and this project adheres to [Semantic Versioning](http://semver.org/).
codebase
- Use host 0.0.0.0 for test servers
- Fix bug with example-flask-peewee import
- Fix bug with example-cherrypy import (Specify parent package name when importing setting, local_setting modules)
- Changed the DB session creation, DB connection closing, and DB model creation to align with SQLAlchemy 2. (for example-flask program)
- Removed flask-script package from requirements.txt as it doesn't work with recent flasks (programs: example-flask, example-flask-mongoengine, example-flask-peewee)
- Configured the manage.py file to use the CLI environment built into flask (programs: example-flask, example-flask-mongoengine, example-flask-peewee) as the flask-script package has been removed
- Changed the part that binds the DB session to comply with SQLAlchemy 2 (for example-pyramid program)
- Changed the part that fetches authenticated users from DB to be SQLAlchemy 2 based (for example-pyramid program)
- Removed all entries in the SOCIAL_AUTH_KEYS dictionary in local_settings.py.template into a single variable (tests showed that adding provider-supplied key information in SOCIAL_AUTH_KEYS caused errors on social login) (for example-pyramid program)
- Changed the DB model creation part. (for example-pyramid program)
- Error occurred when social login integration was completed because it was supposed to find google plus ID in the redirected URL. google plus is discontinued, so delete the part that gets plus ID. (for example-pyramid program)
- Changed the part that binds the DB session and creates the Base Model class to comply with SQLAlchemy 2 (for example-tornado, example-webpy program)
- Added local_settings.py.template file to reference other programs to easily generate local_settings.py (for example-tornado, example-webpy program)
- Fixed an error importing the local_settings module from settings.py (for the example-tornado program)
- Change the declaration of the Model class based on SQLAlchemy 2 (corresponds to the example-tornado, example-webpy program)
- Fix bug with example-webpy import
- Separated AUTHENTICATION_BACKENDS and PIPELINE entries into separate settings.py entries as web.config entries in app.py (for example-webpy program)
23 changes: 13 additions & 10 deletions example-cherrypy/example/app.py
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

Expand All @@ -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")
)
Comment on lines +17 to +19
Copy link

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+.

- DATABASE_NAME = "sqlite:///{dbname}".format(
-     dbname=os.path.join(BASE_DIR, "db.sqlite3")
- )
+ DATABASE_NAME = f"sqlite:///{os.path.join(BASE_DIR, 'db.sqlite3')}"

Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation.

Suggested change
DATABASE_NAME = "sqlite:///{dbname}".format( # fix: skip
dbname=os.path.join(BASE_DIR, "db.sqlite3")
)
DATABASE_NAME = f"sqlite:///{os.path.join(BASE_DIR, 'db.sqlite3')}"


SAEnginePlugin(cherrypy.engine, DATABASE_NAME).subscribe()

Expand All @@ -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
Copy link

Choose a reason for hiding this comment

The 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():
Expand All @@ -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"
)


Expand All @@ -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",
Expand Down
6 changes: 4 additions & 2 deletions example-cherrypy/example/db/__init__.py
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
10 changes: 5 additions & 5 deletions example-cherrypy/example/db/saplugin.py
Original file line number Diff line number Diff line change
@@ -1,13 +1,13 @@
from cherrypy.process import plugins
from sqlalchemy import create_engine
from sqlalchemy.orm import scoped_session, sessionmaker
from sqlalchemy.orm import Session


class SAEnginePlugin(plugins.SimplePlugin):
def __init__(self, bus, connection_string=None):
self.sa_engine = None
self.connection_string = connection_string
self.session = scoped_session(sessionmaker(autoflush=True, autocommit=False))
self.session = Session(autoflush=True, autocommit=False)
super().__init__(bus)

def start(self):
Expand All @@ -23,14 +23,14 @@ def stop(self):
self.sa_engine = None

def bind(self):
self.session.configure(bind=self.sa_engine)
self.session.bind = self.sa_engine
return self.session

def commit(self):
try:
self.session.commit()
except:
except Exception:
self.session.rollback()
raise
finally:
self.session.remove()
self.session.close()
17 changes: 10 additions & 7 deletions example-cherrypy/example/db/user.py
Original file line number Diff line number Diff line change
@@ -1,16 +1,19 @@
from sqlalchemy import Boolean, Column, Integer, String
from typing import Optional

from sqlalchemy import String
from sqlalchemy.orm import Mapped, mapped_column

from . import Base


class User(Base):
__tablename__ = "users"
id = Column(Integer, primary_key=True)
username = Column(String(200))
password = Column(String(200), default="")
name = Column(String(100))
email = Column(String(200))
active = Column(Boolean, default=True)
id: Mapped[int] = mapped_column(primary_key=True)
username: Mapped[str] = mapped_column(String(200))
password: Mapped[str] = mapped_column(String(200), default="")
name: Mapped[Optional[str]] = mapped_column(String(100))
email: Mapped[Optional[str]] = mapped_column(String(200))
active: Mapped[bool] = mapped_column(default=True)

def is_active(self):
return self.active
Expand Down
8 changes: 3 additions & 5 deletions example-cherrypy/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,9 @@
import sys

import cherrypy
from example.app import DATABASE_NAME, run_app
from example.db import Base
from social_cherrypy.models import SocialBase
from sqlalchemy import create_engine

cherrypy.config.update(
Expand All @@ -10,11 +13,6 @@
}
)

from example.app import DATABASE_NAME, run_app
from example.db import Base
from example.db.user import User
from social_cherrypy.models import SocialBase

if __name__ == "__main__":
if len(sys.argv) > 1 and sys.argv[1] == "syncdb":
engine = create_engine(DATABASE_NAME)
Expand Down
8 changes: 6 additions & 2 deletions example-common/filters.py
Original file line number Diff line number Diff line change
Expand Up @@ -42,11 +42,15 @@ def slice_by(value, items):


def social_backends(backends):
return filter_backends(backends, lambda name, backend: name not in LEGACY_NAMES)
return filter_backends( # fix: skip
backends, lambda name, backend: name not in LEGACY_NAMES
)


def legacy_backends(backends):
return filter_backends(backends, lambda name, backend: name in LEGACY_NAMES)
return filter_backends( # fix: skip
backends, lambda name, backend: name in LEGACY_NAMES
)


def oauth_backends(backends):
Expand Down
16 changes: 12 additions & 4 deletions example-common/pipeline.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,11 +11,15 @@ def require_email(strategy, details, user=None, is_new=False, *args, **kwargs):
details["email"] = email
else:
current_partial = kwargs.get("current_partial")
return strategy.redirect(f"/email?partial_token={current_partial.token}")
return strategy.redirect( # fix: skip
f"/email?partial_token={current_partial.token}"
)


@partial
def require_country(strategy, details, user=None, is_new=False, *args, **kwargs):
def require_country( # fix: skip
strategy, details, user=None, is_new=False, *args, **kwargs
):
if kwargs.get("ajax"):
return
elif is_new and not details.get("country"):
Expand All @@ -24,7 +28,9 @@ def require_country(strategy, details, user=None, is_new=False, *args, **kwargs)
details["country"] = country
else:
current_partial = kwargs.get("current_partial")
return strategy.redirect(f"/country?partial_token={current_partial.token}")
return strategy.redirect( # fix: skip
f"/country?partial_token={current_partial.token}"
)


@partial
Expand All @@ -37,4 +43,6 @@ def require_city(strategy, details, user=None, is_new=False, *args, **kwargs):
details["city"] = city
else:
current_partial = kwargs.get("current_partial")
return strategy.redirect(f"/city?partial_token={current_partial.token}")
return strategy.redirect( # fix: skip
f"/city?partial_token={current_partial.token}"
)
4 changes: 3 additions & 1 deletion example-common/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,9 @@ 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
):
"""Common view context"""
context = {
"user": user,
Expand Down
2 changes: 1 addition & 1 deletion example-django-mongoengine/app/admin.py
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.
2 changes: 1 addition & 1 deletion example-django-mongoengine/app/tests.py
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.
5 changes: 1 addition & 4 deletions example-django-mongoengine/app/views.py
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

Copy link

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [71-71]

Detected a potential XSS vulnerability in the ajax_auth function where data is rendered directly to the end user via HttpResponse. To mitigate this risk, consider using Django's template engine to safely render HTML, ensuring that any dynamic content is properly escaped.

Expand Down
2 changes: 1 addition & 1 deletion example-django-mongoengine/example/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -273,6 +273,6 @@
STATIC_URL = "/static/"

try:
from example.local_settings import *
from example.local_settings import * # noqa: F401,F403
except ImportError:
pass
2 changes: 1 addition & 1 deletion example-django-mongoengine/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# issue is really that Django is missing to avoid masking other
# exceptions on Python 2.
try:
import django
import django # noqa: F401
except ImportError:
raise ImportError(
"Couldn't import Django. Are you sure it's installed and "
Expand Down
2 changes: 1 addition & 1 deletion example-django/app/admin.py
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.
2 changes: 1 addition & 1 deletion example-django/app/tests.py
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.
5 changes: 1 addition & 4 deletions example-django/app/views.py
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

Copy link

Choose a reason for hiding this comment

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

📝 NOTE
This review was outside the diff hunks, and no overlapping diff hunk was found. Original lines [97-97]

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: JsonResponse automatically sets the Content-Type header to application/json and safely handles data serialization.

Expand Down
2 changes: 1 addition & 1 deletion example-django/example/settings.py
Original file line number Diff line number Diff line change
Expand Up @@ -272,6 +272,6 @@
STATIC_URL = "/static/"

try:
from example.local_settings import *
from example.local_settings import * # noqa: F401,F403
except ImportError:
pass
2 changes: 1 addition & 1 deletion example-django/manage.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
# issue is really that Django is missing to avoid masking other
# exceptions on Python 2.
try:
import django
import django # noqa: F401
except ImportError:
raise ImportError(
"Couldn't import Django. Are you sure it's installed and "
Expand Down
5 changes: 2 additions & 3 deletions example-flask-mongoengine/example/__init__.py
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,8 @@
from common import filters
from common.utils import common_context
from common.utils import url_for as common_url_for
from flask import Flask, g, url_for
from example import models
from flask import Flask, g
from flask_login import LoginManager, current_user
from flask_mongoengine import MongoEngine
from social_flask.routes import social_auth
Expand Down Expand Up @@ -32,8 +33,6 @@
login_manager.login_message = ""
login_manager.init_app(app)

from example import models, routes


@login_manager.user_loader
def load_user(userid):
Expand Down
4 changes: 2 additions & 2 deletions example-flask-mongoengine/example/models/__init__.py
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
2 changes: 1 addition & 1 deletion example-flask-mongoengine/example/models/user.py
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
from example import db
from flask_login import UserMixin
from mongoengine import BooleanField, EmailField, ListField, ReferenceField, StringField
from mongoengine import BooleanField, EmailField, StringField
from social_flask_mongoengine.models import FlaskStorage


Expand Down
4 changes: 2 additions & 2 deletions example-flask-mongoengine/example/routes/__init__.py
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
2 changes: 0 additions & 2 deletions example-flask-mongoengine/example/settings.py
Original file line number Diff line number Diff line change
@@ -1,5 +1,3 @@
from os.path import abspath, dirname, join

SECRET_KEY = "random-secret-key"
SESSION_COOKIE_NAME = "python_social_auth_mongoengine_session"
DEBUG = True
Expand Down
Loading