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

ESLint fixes for @elastic/kibana-security #195452

Closed
wants to merge 12 commits into from
Closed
Show file tree
Hide file tree
Changes from all 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
58 changes: 58 additions & 0 deletions .github/workflows/security-route-migration.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,58 @@
name: ESLint no_deprecated_authz_config

on:
push:
branches:
- authz-migration-workflow
workflow_dispatch:
inputs:
route_type:
description: 'Choose route type (authorized/unauthorized)'
required: true
default: 'authorized'
type: choice
options:
- authorized
- unauthorized

jobs:
eslint-autofix:
name: Run no_deprecated_authz_config rule
runs-on: ubuntu-latest
permissions:
actions: read
contents: write
security-events: write

env:
ROUTE_TYPE: ${{ github.event.inputs.route_type }}
GH_TOKEN: ${{ secrets.KIBANAMACHINE_TOKEN }}

steps:
- name: Checkout repository
uses: actions/checkout@d632683dd7b4114ad314bca15554477dd762a938 # v4.2.0
with:
ref: ${{ matrix.branch }}

- name: setup node
uses: actions/setup-node@v4
with:
node-version-file: '.nvmrc'

- name: Set GitHub name and email
run: |
git config --global user.name 'kibanamachine'
git config --global user.email '[email protected]'

- name: yarn kbn bootstrap
run: |
yarn kbn bootstrap --no-validate --no-vscode

- name: Install GitHub CLI
run: sudo apt-get install gh -y

# - name: Authenticate with GitHub CLI
# run: gh auth login
- name: Run ESLint Autofix Script
run: node scripts/eslint-security-route.js

1 change: 1 addition & 0 deletions packages/kbn-eslint-config/.eslintrc.js
Original file line number Diff line number Diff line change
Expand Up @@ -326,6 +326,7 @@ module.exports = {
'@kbn/imports/uniform_imports': 'error',
'@kbn/imports/no_unused_imports': 'error',
'@kbn/imports/no_boundary_crossing': 'error',
'kbn/eslint/no_deprecated_authz_config': 'off',

'no-new-func': 'error',
'no-implied-eval': 'error',
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -72,7 +72,7 @@ const maybeReportDisabledSecurityConfig = (node, context, isVersionedRoute = fal
);

const accessTagsFilter = (el) => isLiteralAccessTag(el) || isTemplateLiteralAccessTag(el);
const accessTags = tagsProperty.value.elements.filter(accessTagsFilter);
const accessTags = tagsProperty?.value.elements.filter(accessTagsFilter) ?? [];

return accessTags.length > 0;
}
Expand Down
143 changes: 143 additions & 0 deletions scripts/eslint-security-route.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,143 @@
/*
* Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one
* or more contributor license agreements. Licensed under the "Elastic License
* 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side
* Public License v 1"; you may not use this file except in compliance with, at
* your election, the "Elastic License 2.0", the "GNU Affero General Public
* License v3.0 only", or the "Server Side Public License, v 1".
*/

/* eslint-disable no-restricted-syntax */

const { execSync } = require('child_process');
const fs = require('fs');
const path = require('path');

function runCommand(command) {
try {
return execSync(command, { encoding: 'utf8' }).trim();
} catch (err) {
console.error(`Error running command: ${command}`);
console.error(err.stdout.toString());
process.exit(1);
}
}

function parseCodeOwners(codeownersPath) {
const codeowners = {};
const content = fs.readFileSync(codeownersPath, 'utf8').split('\n');

content.forEach((line) => {
const trimmed = line.trim();
if (!trimmed || trimmed.startsWith('#')) return;

const [pattern, owner] = trimmed.split(/\s+/);
if (pattern && owner) {
codeowners[pattern] = owner;
}
});

return codeowners;
}

function getChangedFiles() {
const diffOutput = runCommand('git diff --name-only --diff-filter=M');
return diffOutput.split('\n').filter(Boolean);
}

function groupFilesByOwners(files, codeowners) {
const ownerFilesMap = {};

files.forEach((file) => {
for (const [pattern, owner] of Object.entries(codeowners)) {
const regexPattern = pattern
.replace(/\*\*/g, '.*')
.replace(/\*/g, '[^/]*')
.replace(/\//g, '\\/');

const regex = new RegExp(`${regexPattern}`);

if (regex.test(file)) {
if (!ownerFilesMap[owner]) ownerFilesMap[owner] = [];
ownerFilesMap[owner].push(file);
break;
}
}
});

return ownerFilesMap;
}

// Create a branch, stage, and commit files for each owner
function processChangesByOwners(ownerFilesMap) {
const mainBranch = execSync('git rev-parse --abbrev-ref HEAD');

for (const [owner, files] of Object.entries(ownerFilesMap)) {
const branchName = `eslint/changes-by-${owner.replace('@elastic/', '')}`;

console.log(`Owner: ${owner}`);
console.log(`Files: ${files.join(', ')} \n ----`);
console.log(`Creating branch: ${branchName}`);

runCommand(`git checkout -b ${branchName}`);

const fileList = files.join(' ');
runCommand(`git add ${fileList}`);
runCommand(`git commit -m "Changes for ${owner}"`);

console.log(`Pushing branch: ${branchName}`);
runCommand(`git push -u origin ${branchName}`);

console.log(`Creating pull request for branch: ${branchName}`);
runCommand(
`gh pr create --base main --head ${branchName} --title "ESLint fixes for ${owner}" --body "This PR contains ESLint fixes for files owned by ${owner}"`
);

runCommand(`git checkout ${mainBranch}`);
}
}

function runESLint() {
console.log(`Running ESLint on ${process.env.ROUTE_TYPE} routes...`);
const eslintRuleFlag =
process.env.ROUTE_TYPE === 'authorized'
? 'MIGRATE_DISABLED_AUTHZ=false'
: 'MIGRATE_DISABLED_AUTHZ=true';

try {
runCommand(
`${eslintRuleFlag} grep -rEl --include="*.ts" "router\.(get|post|delete|put)|router\.versioned\.(get|post|put|delete)" ./x-pack/plugins/security | xargs npx eslint --fix --rule "@kbn/eslint/no_deprecated_authz_config:error"`
);

// runCommand(
// `${eslintRuleFlag} grep -rEl --include="*.ts" "router\.(get|post|delete|put)|router\.versioned\.(get|post|put|delete)" ./x-pack/plugins/banners | xargs npx eslint --fix --rule "@kbn/eslint/no_deprecated_authz_config:error"`
// );
console.log('ESLint autofix complete');
} catch (error) {
console.error('Error running ESLint:', error);
}
}

function main() {
const codeownersPath = path.resolve('.github', 'CODEOWNERS');
if (!fs.existsSync(codeownersPath)) {
console.error('CODEOWNERS file not found');
process.exit(1);
}

const codeowners = parseCodeOwners(codeownersPath);

runESLint();

const changedFiles = getChangedFiles();
if (changedFiles.length === 0) {
console.log('No changes detected.');
return;
}

const ownerFilesMap = groupFilesByOwners(changedFiles, codeowners);

processChangesByOwners(ownerFilesMap);
}

main();
Original file line number Diff line number Diff line change
Expand Up @@ -87,17 +87,17 @@ export function initAPIAuthorization(
const missingPrivileges = Object.keys(kibanaPrivileges).filter(
(key) => !kibanaPrivileges[key]
);
logger.warn(
`User not authorized for "${request.url.pathname}${
request.url.search
}", responding with 403: missing privileges: ${missingPrivileges.join(', ')}`
);
const forbiddenMessage = `API [${request.route.method.toLocaleUpperCase('en')} ${
request.url.pathname
}${
request.url.search
}] is unauthorized for user, this action is granted by the Kibana privileges [${missingPrivileges}]`;

logger.warn(`Responding with 403: ${forbiddenMessage}}`);

return response.forbidden({
body: {
message: `User not authorized for ${request.url.pathname}${
request.url.search
}, missing privileges: ${missingPrivileges.join(', ')}`,
message: forbiddenMessage,
},
});
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,12 @@ export function defineRecordAnalyticsOnAuthTypeRoutes({
router.post(
{
path: '/internal/security/analytics/_record_auth_type',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: {
body: schema.nullable(
schema.object({ signature: schema.string(), timestamp: schema.number() })
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,12 @@ export function defineRecordViolations({ router, analyticsService }: RouteDefini
router.post(
{
path: '/internal/security/analytics/_record_violations',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: {
/**
* Chrome supports CSP3 spec and sends an array of reports. Safari only sends a single
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,16 @@ export function defineAnonymousAccessGetCapabilitiesRoutes({
getAnonymousAccessService,
}: RouteDefinitionParams) {
router.get(
{ path: '/internal/security/anonymous_access/capabilities', validate: false },
{
path: '/internal/security/anonymous_access/capabilities',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: false,
},
async (_context, request, response) => {
const anonymousAccessService = getAnonymousAccessService();
return response.ok({ body: await anonymousAccessService.getCapabilities(request) });
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,7 +18,16 @@ export function defineAnonymousAccessGetStateRoutes({
getAnonymousAccessService,
}: RouteDefinitionParams) {
router.get(
{ path: '/internal/security/anonymous_access/state', validate: false },
{
path: '/internal/security/anonymous_access/state',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: false,
},
async (_context, _request, response) => {
const anonymousAccessService = getAnonymousAccessService();
const accessURLParameters = anonymousAccessService.accessURLParameters
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/create.ts
Original file line number Diff line number Diff line change
Expand Up @@ -32,6 +32,12 @@ export function defineCreateApiKeyRoutes({
router.post(
{
path: '/internal/security/api_key',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: {
body: schema.oneOf([
restApiKeySchema,
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/enabled.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,12 @@ export function defineEnabledApiKeysRoutes({
router.get(
{
path: '/internal/security/api_key/_enabled',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: false,
},
createLicensedRouteHandler(async (context, request, response) => {
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/has_active.ts
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,12 @@ export function defineHasApiKeysRoutes({
router.get(
{
path: '/internal/security/api_key/_has_active',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: false,
options: {
access: 'internal',
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/invalidate.ts
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,12 @@ export function defineInvalidateApiKeysRoutes({ router }: RouteDefinitionParams)
router.post(
{
path: '/internal/security/api_key/invalidate',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: {
body: schema.object({
apiKeys: schema.arrayOf(schema.object({ id: schema.string(), name: schema.string() })),
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/query.ts
Original file line number Diff line number Diff line change
Expand Up @@ -25,6 +25,12 @@ export function defineQueryApiKeysAndAggregationsRoute({
// on behalf of the user making the request and governed by the user's own cluster privileges.
{
path: '/internal/security/api_key/_query',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: {
body: schema.object({
query: schema.maybe(schema.object({}, { unknowns: 'allow' })),
Expand Down
6 changes: 6 additions & 0 deletions x-pack/plugins/security/server/routes/api_keys/update.ts
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,12 @@ export function defineUpdateApiKeyRoutes({
router.put(
{
path: '/internal/security/api_key',
security: {
authz: {
enabled: false,
reason: 'This route is opted out from authorization',
},
},
validate: {
body: schema.oneOf([
updateRestApiKeySchema,
Expand Down
Loading