-
Notifications
You must be signed in to change notification settings - Fork 0
Added authentication on websocket connections #4
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
base: feature-redis-persistence-support
Are you sure you want to change the base?
Changes from all commits
d88ad1e
a9607dd
6e89858
2d005e9
95f0dfb
6c7bb05
903a337
a741bc3
f192083
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,3 +1,5 @@ | ||
| dist | ||
| node_modules | ||
| tmp | ||
| tmp | ||
| tests | ||
| db/ |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,53 @@ | ||
| const authCallback = process.env.YWEBSOCKET_HTTP_AUTH_CALLBACK || 'http://localhost/auth/' | ||
| const authCallbackParam = process.env.YWEBSOCKET_HTTP_AUTH_CALLBACK_GET_PARAM || 'code' | ||
| const querystring = require('querystring') | ||
|
|
||
| module.exports = { | ||
| authenticate: function (request) { | ||
| return new Promise(function (resolve, reject) { | ||
| const docName = request.url.substring(1) | ||
| const query = querystring.stringify({ | ||
| [authCallbackParam]: docName | ||
| }) | ||
| let authRequester | ||
| if (authCallback.indexOf('https:') === 0) { | ||
| authRequester = require('https') | ||
| } else { | ||
| authRequester = require('http') | ||
| } | ||
| const authCallbackWithRoomCode = authCallback + '?' + query | ||
| console.log(authCallbackWithRoomCode) | ||
| const authRequest = authRequester.request( | ||
| authCallbackWithRoomCode, | ||
| { | ||
| method: 'GET', | ||
| headers: { | ||
| Cookie: request.headers.cookie || '' | ||
| } | ||
| }, | ||
| response => { | ||
| if (response.statusCode < 200 || response.statusCode >= 300) { | ||
| return reject(new Error('statusCode=' + response.statusCode)) | ||
| } | ||
| response.setEncoding('utf8') | ||
| let rawData = '' | ||
| response.on('data', chunk => { | ||
| console.log(chunk) | ||
| rawData += chunk | ||
| }) | ||
| response.on('end', () => { | ||
| const data = JSON.parse(rawData) | ||
| console.log(rawData) | ||
| if (data.status === 'ok') { | ||
| resolve(true) | ||
| } | ||
| }) | ||
| } | ||
| ) | ||
| authRequest.on('error', function (error) { | ||
| reject(error) | ||
| }) | ||
| authRequest.end() | ||
| }) | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,23 @@ | ||
| const mysql = require('mysql2') | ||
|
|
||
| const MYSQL_HOST = process.env.MYSQL_HOST || 'localhost' | ||
| const MYSQL_PORT = process.env.MYSQL_PORT || 3306 | ||
| const MYSQL_USER = process.env.MYSQL_USER || 'root' | ||
| const MYSQL_PASSWORD = process.env.MYSQL_PASSWORD || '' | ||
| const MYSQL_DATABASE = process.env.MYSQL_DATABASE || 'database' | ||
|
|
||
| const mysqlConnection = mysql.createConnection({ | ||
| host: MYSQL_HOST, | ||
| user: MYSQL_USER, | ||
| database: MYSQL_DATABASE, | ||
| password: MYSQL_PASSWORD, | ||
| port: MYSQL_PORT | ||
| }) | ||
|
|
||
| mysqlConnection.connect(function (error) { | ||
| if (error) throw error | ||
| }) | ||
|
|
||
| module.exports = { | ||
| mysqlConnection: mysqlConnection | ||
| } |
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -1,29 +1,11 @@ | ||
| #!/usr/bin/env node | ||
|
|
||
| const redisPersistenceBridge = require('./redis-persistence-bridge') | ||
| const mysqlConnection = require('../connections/mysql').mysqlConnection | ||
|
Member
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. Предполагаю, что это было вытащено для авторизации через django_session? Думаю, сейчас можно втащить обратно. Кажется, ни к чему плодить кучи файлов, не?
Collaborator
Author
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. Опять-же, потому что это библиотека, а у кого-то может быть Postgres или Mongo.
Member
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. Если у него Postgres или Mongo, то и скрипт будет называться transfer-data-from-postrgres-или-mongo-to-redis. И SQL-запросы в этом скрипте будут другие. Короче, я к тому, что этот скрипт сам по себе очень специфичен для mysql и не вижу причин делить его на два файла. Либо надо его тоже абстрагировать: К слову, раз мы вернули level-db, надо, видимо, и эту механику адаптировать, м? |
||
|
|
||
| const mysql = require('mysql2') | ||
|
|
||
| const MYSQL_HOST = process.env.MYSQL_HOST || 'localhost' | ||
| const MYSQL_USER = process.env.MYSQL_USER || 'root' | ||
| const MYSQL_PASSWORD = process.env.MYSQL_PASSWORD || '' | ||
| const MYSQL_DATABASE = process.env.MYSQL_DATABASE || 'database' | ||
| const MYSQL_TABLE = process.env.MYSQL_TABLE || 'table' | ||
| const MYSQL_CONTENT_FIELDS = process.env.MYSQL_CONTENT_FIELDS || 'content' | ||
| const MYSQL_KEY_FIELD = process.env.MYSQL_KEY_FIELD || 'id' | ||
| const MYSQL_PORT = process.env.MYSQL_PORT || 3306 | ||
|
|
||
| const mysqlConnection = mysql.createConnection({ | ||
| host: MYSQL_HOST, | ||
| user: MYSQL_USER, | ||
| database: MYSQL_DATABASE, | ||
| password: MYSQL_PASSWORD, | ||
| port: MYSQL_PORT | ||
| }) | ||
|
|
||
| mysqlConnection.connect(function (error) { | ||
| if (error) throw error | ||
| }) | ||
|
|
||
| mysqlConnection.query( | ||
| `SELECT ${MYSQL_KEY_FIELD}, ${MYSQL_CONTENT_FIELDS} FROM ${MYSQL_TABLE}`, | ||
|
|
||
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.
Если честно, довольно непонятно, что именно тут происходит.
Непонятно, что за
request.url.substring(1). Предполагаю, что это накладывает определённые условия на request.url? Какойurlожидается изначально?Что должно храниться в
authCallbackи что в итоге получится в результатеauthCallback + docName? Получается, чтоauthCallbackтоже должно быть очень определённого формата, так?Плюс тут, кажется, есть возможность устроить какой-нибудь хак, типа, негодяй в
request.urlпередаст что-то вроде../../../api/v1/something-that-always-returns-status-ok/и получит какой-то несанкционированный доступ.Короче, напрягает, что тут происходит что-то, что сильно полагается на символы в строчках, и что весь процесс никак не прокомментирован, а из кода понять — сложно.
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.
request.url тут всегда docName, если умышленно не передать что-то другое. Если какой-то негодяй передаст что угодно,то это что угодно просто передастся get-параметром к авторизационному урлу, то получится, просто левое значение.
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.
А как бы ты передал значение docName?
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.
Написал вот в этом комменте. По сути на самом деле так же, конечно, но слегка аккуратнее.
Тут я бы предложил вынести в функцию, чтобы её же задокументировать, и чтобы в логику не просачивались детали реализации, а если в ней будем обнаруживать баги (например, вдруг в каких-то условиях в request.url передастся полный урл?), то там усложним и тестами покроем. Ну или абстрагируем вовсе. Но в любом случае будущее — это про YAGNI, а в настоящем важны документирование и семантика:
(вот тут бы питоновские доктесты были бы идеальны :–)
А ещё я не нашёл в документации
request.url. Куда смотреть? Хочу убедиться, что он там действительно относительный, и мы не окажемся сdocNameвида"ttps://...:":–)Может, уместнее будет использовать
request.path?