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

New Feature - Authentication Middleware Injection #123

Open
wants to merge 9 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 7 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
1 change: 1 addition & 0 deletions .env.schema
Original file line number Diff line number Diff line change
Expand Up @@ -12,3 +12,4 @@ LOG_LEVEL=
PORT=3030
UPLOAD_LIMIT=''
PLURALIZE_SCHEMAS_ENABLED=
ALLOWED_ORIGINS=
1 change: 1 addition & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -88,6 +88,7 @@ The Environment Variables used for this application are listed in the table bell
| `PLURALIZE_SCHEMAS_ENABLED` | This feature automatically convert schema names to their plural forms when handling compound documents. Pluralization assumes the words are in English | true |
| `PORT` | Server Port. | 3030 |
| `UPLOAD_LIMIT` | Limit upload file size in string or number. <br>Supported units and abbreviations are as follows and are case-insensitive: <br> - b for bytes<br> - kb for kilobytes<br>- mb for megabytes<br>- gb for gigabytes<br>- tb for terabytes<br>- pb for petabytes<br>Any other text is considered as byte | '10mb' |
| `ALLOWED_ORIGINS` | Specifies a list of permitted origins for Cross-Origin Resource Sharing (CORS). These origins, separated by commas, are allowed to make requests to the server, ensuring only trusted domains can access resources. | |

Choose a reason for hiding this comment

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

Example value could be helpful here


## Script commands (Workspace)

Expand Down
2 changes: 2 additions & 0 deletions apps/server/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -23,6 +23,7 @@
"private": true,
"dependencies": {
"@overture-stack/lyric": "workspace:^",
"cors": "^2.8.5",
"dotenv": "^16.4.5",
"express": "^4.19.2",
"helmet": "^7.1.0",
Expand All @@ -31,6 +32,7 @@
"winston": "^3.13.1"
},
"devDependencies": {
"@types/cors": "^2.8.17",
"@types/express": "^4.17.21",
"@types/express-serve-static-core": "^4.19.5",
"@types/qs": "^6.9.15",
Expand Down
21 changes: 21 additions & 0 deletions apps/server/src/auth/handler.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,21 @@
import { Request } from 'express';

import { type UserSessionResult } from '@overture-stack/lyric';

/**
* Function to implement authentication logic.
* This function is called by the authMiddleware to verify the user's authentication.
* It returns the authentication result, which includes the authentication status and user information.
*
* @param req - Express request object
* @returns User session result
*/
export const authHandler = (_req: Request) => {
Copy link
Contributor Author

@leoraba leoraba Jan 22, 2025

Choose a reason for hiding this comment

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

This is the implementation of an auth handler used to provider Guest access.

Choose a reason for hiding this comment

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

I don't know the whole project context, so I'm wondering why the readme example for authHandler contains actual auth logic, but the actual authHandler implementation just returns a hardcoded value

Choose a reason for hiding this comment

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

I think the answer is related to this? #123 (comment)

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The handler implemented in this apps/server package is a basic implementation for demonstration, however, the details how to customize it is described in readme file

// Guest User Session
const authResult: UserSessionResult = {
user: { username: 'Guest' },
authStatus: 'authenticated',
};

return authResult;
};
Original file line number Diff line number Diff line change
Expand Up @@ -2,9 +2,12 @@ import 'dotenv/config';

import { AppConfig } from '@overture-stack/lyric';

import { authHandler } from '../auth/handler.js';

export const getServerConfig = () => {
return {
port: process.env.PORT || 3030,
allowedOrigins: process.env.ALLOWED_ORIGINS,
};
};

Expand All @@ -27,7 +30,7 @@ const getRequiredConfig = (name: string) => {
return value;
};

export const defaultAppConfig: AppConfig = {
export const appConfig: AppConfig = {
db: {
host: getRequiredConfig('DB_HOST'),
port: Number(getRequiredConfig('DB_PORT')),
Expand Down Expand Up @@ -57,4 +60,8 @@ export const defaultAppConfig: AppConfig = {
logger: {
level: process.env.LOG_LEVEL || 'info',
},
auth: {
enabled: getBoolean(process.env.AUTH_ENABLED, true),
customAuthHandler: authHandler,
},
};
27 changes: 22 additions & 5 deletions apps/server/src/server.ts
Original file line number Diff line number Diff line change
@@ -1,23 +1,40 @@
import cors from 'cors';
import express from 'express';
import helmet from 'helmet';
import { serve, setup } from 'swagger-ui-express';

import { errorHandler, provider } from '@overture-stack/lyric';

import { defaultAppConfig, getServerConfig } from './config/server.js';
import { appConfig, getServerConfig } from './config/app.js';
import swaggerDoc from './config/swagger.js';
import healthRouter from './routes/health.js';
import pingRouter from './routes/ping.js';

const serverConfig = getServerConfig();
const { allowedOrigins, port } = getServerConfig();

const lyricProvider = provider(defaultAppConfig);
const lyricProvider = provider(appConfig);

// Create Express server
const app = express();

app.use(helmet());

app.use(
cors({
origin: function (origin, callback) {
if (!origin) {
// allow requests with no origin
// (like mobile apps or curl requests)
return callback(null, true);
Copy link
Member

Choose a reason for hiding this comment

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

do we want to add a feature flag (env var config) to enable this, and have it disabled by default?
doesn't feel right to just have it "open" like this

Copy link
Contributor Author

@leoraba leoraba Jan 29, 2025

Choose a reason for hiding this comment

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

Thanks for noticing this. In regards to security this should be disabled by default.
And could be converted into a variable named CORS_ENABLED where when true only registered domains can access the server, otherwise rejected (default)

} else if (allowedOrigins && allowedOrigins.split(',').indexOf(origin) !== -1) {
return callback(null, true);
}
const msg = 'The CORS policy for this site does not allow access from the specified Origin.';
return callback(new Error(msg), false);
},
}),
);

// Ping Route
app.use('/ping', pingRouter);

Expand All @@ -35,6 +52,6 @@ app.use('/health', healthRouter);

app.use(errorHandler);
// running the server
app.listen(serverConfig.port, () => {
console.log(`Starting Express server on http://localhost:${serverConfig.port}`);
app.listen(port, () => {
console.log(`Starting Express server on http://localhost:${port}`);
justincorrigible marked this conversation as resolved.
Show resolved Hide resolved
});
120 changes: 105 additions & 15 deletions packages/data-provider/README.md
Copy link
Contributor Author

Choose a reason for hiding this comment

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

Updating Docs. Adding section to implement auth middleware and database migrations

Copy link
Member

@justincorrigible justincorrigible Jan 29, 2025

Choose a reason for hiding this comment

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

those examples are really helpful! wonder if it would be more useful for integrators to have those as actual files instead (like a ready to copy and customize scaffolding)

thoughts?

Copy link
Contributor Author

@leoraba leoraba Jan 29, 2025

Choose a reason for hiding this comment

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

I think providing our database migration script allows integrators to ensure Lyric will keep track of codebase and database schema in sync. For example, integrators over time can jump to latest version of Lyric, and they will be sure the database schema will be updated as well. In essence, how Lyric stores data shouldn't be a headache for implementers.

Original file line number Diff line number Diff line change
Expand Up @@ -8,56 +8,146 @@
npm i @overture-stack/lyric
```

## Usage
## Configuration

### Provider

Import `AppConfig` and `provider` from `@overture-stack/lyric` module to initialize the provider with custom configuration:

```
```javascript
import { AppConfig, provider } from '@overture-stack/lyric';

const appConfig: AppConfig = {
db: {
host: [INSERT_DB_HOST],
port: [INSERT_DB_PORT],
database: [INSERT_DB_NAME],
user:[INSERT_DB_USER],
password: [INSERT_DB_PASSWORD],
host: 'localhost', // Database hostname or IP address
port: 5432, // Database port
database: 'my_database', // Name of the database
user: 'db_user', // Username for database authentication
password: 'secure_password', // Password for database authentication
},
features: {
audit: {
enabled: [INSERT_AUDIT_ENABLED]
}
enabled: true, // Enable audit functionality (true/false)
},
recordHierarchy: {
pluralizeSchemasName: false, // Enable or disable automatic schema name pluralization (true/false)
},
},
schemaService: {
url: [INSERT_LECTERN_URL],
idService: {
useLocal: true, // Use local ID generation (true/false)
customAlphabet: 'ABCDEFGHIJKLMNOPQRSTUVWXYZ1234567890', // Custom alphabet for ID generation
customSize: 12, // Size of the generated ID
},
limits: {
fileSize: [INSERT_UPLOAD_LIMIT],
fileSize: 10485760, // Maximum file size in bytes (e.g., 10MB = 10 * 1024 * 1024)
},
logger: {
level: [INSERT_LOG_LEVEL],
level: 'info', // Logging level (e.g., 'debug', 'info', 'warn', 'error')
},
schemaService: {
url: 'https://api.lectern-service.com', // URL of the schema service
},
};


const lyricProvider = provider(appConfig);
```

Use any of the resources available on provider on a Express server:
### Auth Custom Hanlder

Choose a reason for hiding this comment

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

Typo, should be Handler


The **authentication custom handler** is a customized function that can be used to verify and manage user authentication within the application. It is used by the auth middleware to process incoming requests.

- Import a router:
The handler returns a `UserSessionResult` response type, which is a structured object that indicates the authentication status (`authenticated`, `no-auth`, or `invalid-auth`), if the status is `authenticated` it also include user details provided by the `UserSession` type.

Example how to implement a custom auth handler:

```javascript
import { Request } from 'express';
import { UserSessionResult } from '@overture-stack/lyric';
import jwt from 'jsonwebtoken';

const authHandler = (req: Request): UserSessionResult => {
// Extract the token from the request header
const authHeader = req.headers['authorization'];

// Check if the Authorization header exists and starts with "Bearer"
if (!authHeader || !authHeader.startsWith('Bearer ')) {
return {
authStatus: 'no-auth'
}
}

// Extract the token by removing the "Bearer " prefix
const token = authHeader.split(' ')[1];

try {
// Verify the token using a public key
const publicKey = process.env.JWT_PUBLIC_KEY!;
const decodedToken = jwt.verify(token, publicKey);

return {
user: { username: decodedToken.username }, // Example: Adjust fields as per your `UserSession` type
authStatus: 'authenticated',
};
} catch (err) {
return {
authStatus: 'invalid-auth';
}
}
};
```

Add the handler to your `AppConfig` object.

```javascript
import { AppConfig, provider, UserSession } from '@overture-stack/lyric';

const appConfig: AppConfig = {
...// Other configuration
auth: {
enabled: true,
customAuthHandler: authHandler,
};
}
```

## Usage

### Express Routers

Use any of the resources available on provider on a Express server:

```javascript
import express from 'express';

const app = express();

app.use('/submission', lyricProvider.routers.submission);
```

### Database Migrations

Import `migrate` function from `@overture-stack/lyric` module to run Database migrations

```javascript
import { migrate } from '@overture-stack/lyric';

migrate({
host: 'localhost', // Database hostname or IP address
port: 5432, // Database port
database: 'my_database', // Name of the database
user: 'db_user', // Username for database authentication
password: 'secure_password', // Password for database authentication
});
```

## Support & Contributions

- Developer documentation [docs](https://github.com/overture-stack/lyric/blob/main/packages/data-provider/docs/add-new-resources.md)
- Filing an [issue](https://github.com/overture-stack/lyric/issues)
- Connect with us on [Slack](http://slack.overture.bio)
- Add or Upvote a [feature request](https://github.com/overture-stack/lyric/issues/new?assignees=&labels=&projects=&template=Feature_Requests.md)

```

```

Choose a reason for hiding this comment

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

This code block looks unused?

1 change: 1 addition & 0 deletions packages/data-provider/index.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,7 @@
// config
export { type AppConfig } from './src/config/config.js';
export { default as provider } from './src/core/provider.js';
export { type AuthStatus, type UserSession, type UserSessionResult } from './src/middleware/auth.js';
export { errorHandler } from './src/middleware/errorHandler.js';
export { type DbConfig, migrate } from '@overture-stack/lyric-data-model';

Expand Down
2 changes: 2 additions & 0 deletions packages/data-provider/src/config/config.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ import { NodePgDatabase } from 'drizzle-orm/node-postgres';
import type { DbConfig } from '@overture-stack/lyric-data-model';
import * as schema from '@overture-stack/lyric-data-model/models';

import type { AuthConfig } from '../middleware/auth.js';
import { Logger } from './logger.js';

export type AuditConfig = {
Expand Down Expand Up @@ -48,6 +49,7 @@ export type AppConfig = {
limits: LimitsConfig;
logger: LoggerConfig;
schemaService: SchemaServiceConfig;
auth: AuthConfig;
};

/**
Expand Down
Loading