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

[BUG] salt-api always takes * ACL in configuration instead of acl method from rest.py #62022

Closed
vkaylee opened this issue May 4, 2022 · 1 comment · Fixed by #62680
Closed
Labels
Bug broken, incorrect, or confusing behavior needs-triage Salt-API security issues and PRs for the Security Working Group

Comments

@vkaylee
Copy link

vkaylee commented May 4, 2022

Description
When the external_auth is rest
Instead of taking permission ACL from acl method from rest.py. It always takes the * permission in the configuration

Setup

Simple auth rest API on localhost:3000/api/login

const express = require('express')
const app = express()
const port = 3000
app.use(express.urlencoded({ extended: true }))
app.use(express.json())

app.post('/salt/api/auth', (req, res) => {
    let username = req.body.username
    console.log("username:" + username)

    if ( username === '[email protected]' )
    {
        // Return the list of permissions
        res.status(200).send([
            ".*",
            "@runner",
            "@wheel",
            "@jobs"
        ])
        return
    }
    res.status(404).send([])
})

app.use(function(req, res) {
    res.status(404).send({url: req.originalUrl + ' not found'})
})

app.listen(port)

console.log('RESTful API server started on: ' + port)

The restful API return the list of permissions

[
    ".*",
    "@runner",
    "@wheel",
    "@jobs"
]

In /srv/configs/master/api.conf

rest_cherrypy:
  port: 8000
  root_prefix: /api/
  disable_ssl: True
  socket_queue_size: 100
  thread_pool: 200
  collect_stats: True

In /srv/configs/master/api.conf

external_auth:
  rest:
    ^url: localhost:3000/salt/api/auth
    # Test permissions
    '[email protected]':
      - centOS-*:
        - cmd.run_all
        - 'cp.get_url':
            kwargs:
              'path': 'https://example.com/.*'
    # Default permissions
    '*':
      - default-*:
        - grains.items
        - test.ping
        - test.arg
        - status.uptime
        - cmd.run_all
        - cmd.script:
            kwargs:
              'source': 'https://google.com/.*'

Steps to Reproduce the behavior
Use curl tool to post login request

curl -X POST http://127.0.0.1:8000/api/login -H "Content-Type: application/json" -d '{"eauth":"rest","username":"[email protected]","password":"ac230fc3-cf42-4b04-a00b-de1ca906a830"}'

Json return

{
  "return": [
    {
      "token": "ee6b5dac954acaa064362b779c115b7e6afdcd7c",
      "expire": 1651700093.4160972,
      "start": 1651656893.4160972,
      "user": "[email protected]",
      "eauth": "rest",
      "perms": [
        {
          "centOS-*": [
            "cmd.run_all",
            {
              "cp.get_url": {
                "kwargs": {
                  "path": "https://example.com/.*"
                }
              }
            }
          ]
        },
        {
          "default-*": [
            "grains.items",
            "test.ping",
            "test.arg",
            "status.uptime",
            "cmd.run_all",
            {
              "cmd.script": {
                "kwargs": {
                  "source": "https://google.com/.*"
                }
              }
            }
          ]
        }
      ]
    }
  ]
}

Expected behavior
Json return

{
  "return": [
    {
      "token": "a396b600bcb1e7581093c08ab35811a6450c96fa",
      "expire": 1651700575.2555366,
      "start": 1651657375.2555366,
      "user": "[email protected]",
      "eauth": "rest",
      "perms": [
        {
          "centOS-*": [
            "cmd.run_all",
            {
              "cp.get_url": {
                "kwargs": {
                  "path": "https://example.com/.*"
                }
              }
            }
          ]
        },
        ".*",
        "@runner",
        "@wheel",
        "@jobs"
      ]
    }
  ]
}

Screenshots
If applicable, add screenshots to help explain your problem.

Versions Report

salt --versions-report (Provided by running salt --versions-report. Please also mention any differences in master/minion versions.)
Salt Version:
          Salt: 3004.1
 
Dependency Versions:
          cffi: 1.15.0
      cherrypy: unknown
      dateutil: 2.8.1
     docker-py: Not Installed
         gitdb: Not Installed
     gitpython: Not Installed
        Jinja2: 2.11.3
       libgit2: Not Installed
      M2Crypto: Not Installed
          Mako: Not Installed
       msgpack: 1.0.0
  msgpack-pure: Not Installed
  mysql-python: Not Installed
     pycparser: 2.21
      pycrypto: Not Installed
  pycryptodome: 3.9.7
        pygit2: Not Installed
        Python: 3.9.2 (default, Feb 28 2021, 17:03:44)
  python-gnupg: Not Installed
        PyYAML: 5.3.1
         PyZMQ: 20.0.0
         smmap: Not Installed
       timelib: Not Installed
       Tornado: 4.5.3
           ZMQ: 4.3.4
 
System Versions:
          dist: debian 11 bullseye
        locale: utf-8
       machine: x86_64
       release: 5.14.0-1032-oem
        system: Linux
       version: Debian GNU/Linux 11 bullseye

Additional context
The current acl method in rest.py

def acl(username, **kwargs):
    """
    REST authorization
    """
    salt_eauth_acl = __opts__["external_auth"]["rest"].get(username, [])
    log.debug("acl from salt for user %s: %s", username, salt_eauth_acl)

    # Get ACL from REST API
    eauth_rest_acl = []
    result = fetch(username, kwargs["password"])
    if result:
        eauth_rest_acl = result
        log.debug("acl from rest for user %s: %s", username, eauth_rest_acl)

    merged_acl = salt_eauth_acl + eauth_rest_acl

    log.debug("acl from salt and rest merged for user %s: %s", username, merged_acl)
    # We have to make the .get's above return [] since we can't merge a
    # possible list and None. So if merged_acl is falsey we return None so
    # other eauth's can return an acl.
    if not merged_acl:
        return None
    else:
        return merged_acl
@vkaylee vkaylee added Bug broken, incorrect, or confusing behavior needs-triage labels May 4, 2022
@welcome
Copy link

welcome bot commented May 4, 2022

Hi there! Welcome to the Salt Community! Thank you for making your first contribution. We have a lengthy process for issues and PRs. Someone from the Core Team will follow up as soon as possible. In the meantime, here’s some information that may help as you continue your Salt journey.
Please be sure to review our Code of Conduct. Also, check out some of our community resources including:

There are lots of ways to get involved in our community. Every month, there are around a dozen opportunities to meet with other contributors and the Salt Core team and collaborate in real time. The best way to keep track is by subscribing to the Salt Community Events Calendar.
If you have additional questions, email us at [email protected]. We’re glad you’ve joined our community and look forward to doing awesome things with you!

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Bug broken, incorrect, or confusing behavior needs-triage Salt-API security issues and PRs for the Security Working Group
Projects
None yet
2 participants