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

Add and populate user on cronjobs and creator on triggers (SYN-8229, SYN-6850) #4043

Open
wants to merge 22 commits into
base: master
Choose a base branch
from
Open
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
5 changes: 5 additions & 0 deletions changes/130b3a2b54c9b9df3361040576a07864.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
desc: Added a 'creator' value to trigger definitions.
prs: []
type: feat
...
7 changes: 7 additions & 0 deletions changes/aa06e007feb5f2448d8677080b4b4ceb.yaml
Original file line number Diff line number Diff line change
@@ -0,0 +1,7 @@
---
desc: Added a 'user' value to cronjob definitions, which is now used to control the
user the job will be run as rather than the 'creator' value. The 'creator' value
may no longer be modified.
prs: []
type: feat
...
76 changes: 71 additions & 5 deletions synapse/cortex.py
Original file line number Diff line number Diff line change
Expand Up @@ -1095,6 +1095,49 @@ async def _viewNomergeToProtected(self):
await view.setViewInfo('protected', nomerge)
await view.setViewInfo('nomerge', None)

async def _migrateCronUser(self):
for iden, cron in self.agenda.list():
await self.editCronJob(iden, 'user', cron.creator)

oldperm = ('cron', 'set', 'creator')
newperm = ('cron', 'set', 'user')

for user in self.auth.users():
rules = []
update = False

for allow, path in user.getRules():
if path[:3] == oldperm:
update = True
rules.append((allow, newperm + path[3:]))
continue
rules.append((allow, path))

if update:
await user.setRules(rules)

for role in self.auth.roles():
rules = []
update = False

for allow, path in role.getRules():
if path[:3] == oldperm:
update = True
rules.append((allow, newperm + path[3:]))
continue
rules.append((allow, path))

if update:
await role.setRules(rules)

@s_nexus.Pusher.onPushAuto('cortex:migr:trigger:creator')
async def _migrateTriggerCreator(self):
for view in self.views.values():
for iden, trig in view.triggers.list():
if trig.tdef.get('creator') is None:
trig.tdef['creator'] = trig.tdef['user']
view.trigdict.set(iden, trig.tdef)

async def _storUpdateMacros(self):
for name, node in await self.hive.open(('cortex', 'storm', 'macros')):

Expand Down Expand Up @@ -1526,6 +1569,8 @@ async def _execCellUpdates(self):
await self._bumpCellVers('cortex:defaults', (
(1, self._addAllLayrRead),
(2, self._viewNomergeToProtected),
(3, self._migrateCronUser),
(4, self._migrateTriggerCreator),
))

async def _addAllLayrRead(self):
Expand Down Expand Up @@ -6536,7 +6581,7 @@ async def addCronJob(self, cdef):

cdef['created'] = s_common.now()

opts = {'user': cdef['creator'], 'view': cdef.get('view')}
opts = {'user': cdef['user'], 'view': cdef.get('view')}

view = self._viewFromOpts(opts)
cdef['view'] = view.iden
Expand All @@ -6554,7 +6599,8 @@ async def _onAddCronJob(self, cdef):

self.auth.reqNoAuthGate(iden)

user = await self.auth.reqUser(cdef['creator'])
useriden = cdef.get('user', cdef['creator'])
user = await self.auth.reqUser(useriden)

cdef = await self.agenda.add(cdef)

Expand Down Expand Up @@ -6664,24 +6710,44 @@ async def listCronJobs(self):

info = cron.pack()

user = self.auth.user(cron.creator)
user = self.auth.user(cron.user)
if user is not None:
info['username'] = user.name

creator = self.auth.user(cron.creator)
if creator is not None:
info['creatorname'] = creator.name

crons.append(info)

return crons

@s_nexus.Pusher.onPushAuto('cron:edit')
async def editCronJob(self, iden, name, valu):

if name == 'user':
await self.auth.reqUser(valu)

elif name not in ('name', 'doc', 'pool'):
mesg = f'editCronJob name {name} is not supported for editing.'
raise s_exc.BadArg(mesg=mesg)

return await self._push('cron:edit', iden, name, valu)

@s_nexus.Pusher.onPush('cron:edit')
async def _editCronJob(self, iden, name, valu):
'''
Modify a cron job definition.
'''
appt = await self.agenda.get(iden)
# TODO make this generic and check cdef

if name == 'creator':
if name == 'user':
await self.auth.reqUser(valu)
appt.user = valu

elif name == 'creator':
await self.auth.reqUser(valu)
appt.user = valu
appt.creator = valu

elif name == 'name':
Expand Down
42 changes: 26 additions & 16 deletions synapse/lib/agenda.py
Original file line number Diff line number Diff line change
Expand Up @@ -269,7 +269,7 @@
'lastfinishtime',
}

def __init__(self, stor, iden, recur, indx, query, creator, recs, nexttime=None, view=None, created=None, pool=False):
def __init__(self, stor, iden, recur, indx, query, creator, user, recs, nexttime=None, view=None, created=None, pool=False):
self.doc = ''
self.name = ''
self.task = None
Expand All @@ -279,7 +279,8 @@
self.recur = recur # does this appointment repeat
self.indx = indx # incremented for each appt added ever. Used for nexttime tiebreaking for stable ordering
self.query = query # query to run
self.creator = creator # user iden to run query as
self.user = user # user iden to run query as
self.creator = creator # user iden which created the appt
self.recs = recs # List[ApptRec] list of the individual entries to calculate next time from
self._recidxnexttime = None # index of rec who is up next
self.view = view
Expand Down Expand Up @@ -346,6 +347,7 @@
'view': self.view,
'indx': self.indx,
'query': self.query,
'user': self.user,
'creator': self.creator,
'created': self.created,
'recs': [d.pack() for d in self.recs],
Expand All @@ -364,7 +366,11 @@
if val['ver'] != 1:
raise s_exc.BadStorageVersion(mesg=f"Found version {val['ver']}") # pragma: no cover
recs = [ApptRec.unpack(tupl) for tupl in val['recs']]
appt = cls(stor, val['iden'], val['recur'], val['indx'], val['query'], val['creator'], recs, nexttime=val['nexttime'], view=val.get('view'))

creator = val['creator']
user = val.get('user', creator)

appt = cls(stor, val['iden'], val['recur'], val['indx'], val['query'], creator, user, recs, nexttime=val['nexttime'], view=val.get('view'))
appt.doc = val.get('doc', '')
appt.name = val.get('name', '')
appt.pool = val.get('pool', False)
Expand Down Expand Up @@ -526,7 +532,10 @@
The cron definition may contain the following keys:

creator (str)
Iden of the creating user.
Iden of the user which created the appointment.

user (str)
Iden of the user used to run the Storm query.

iden (str)
Iden of the appointment.
Expand Down Expand Up @@ -556,9 +565,10 @@
incvals = cdef.get('incvals')
reqs = cdef.get('reqs', {})
query = cdef.get('storm')
creator = cdef.get('creator')
view = cdef.get('view')
created = cdef.get('created')
creator = cdef.get('creator')
user = cdef.get('user', creator)

pool = cdef.get('pool', False)

Expand All @@ -575,8 +585,8 @@

await self.core.getStormQuery(query)

if not creator:
raise ValueError('"creator" key is cdef parameter is not present or empty')
if not user:
raise ValueError('"user" key is cdef parameter is not present or empty')

if not reqs and incunit is None:
raise ValueError('at least one of reqs and incunit must be non-empty')
Expand All @@ -603,7 +613,7 @@
incvals = (incvals, )
recs.extend(ApptRec(rd, incunit, v) for (rd, v) in itertools.product(reqdicts, incvals))

appt = _Appt(self, iden, recur, indx, query, creator, recs, nexttime=nexttime, view=view, created=created, pool=pool)
appt = _Appt(self, iden, recur, indx, query, creator, user, recs, nexttime=nexttime, view=view, created=created, pool=pool)
self._addappt(iden, appt)

appt.doc = cdef.get('doc', '')
Expand Down Expand Up @@ -751,8 +761,8 @@
try:
await self._execute(appt)
except Exception as e:
extra = {'iden': appt.iden, 'name': appt.name, 'user': appt.creator, 'view': appt.view}
user = self.core.auth.user(appt.creator)
extra = {'iden': appt.iden, 'name': appt.name, 'user': appt.user, 'view': appt.view}
user = self.core.auth.user(appt.user)
if user is not None:
extra['username'] = user.name
if isinstance(e, s_exc.SynErr):
Expand All @@ -767,25 +777,25 @@
'''
Fire off the task to make the storm query
'''
user = self.core.auth.user(appt.creator)
user = self.core.auth.user(appt.user)
if user is None:
logger.warning(f'Unknown user {appt.creator} in stored appointment {appt.iden} {appt.name}',
extra={'synapse': {'iden': appt.iden, 'name': appt.name, 'user': appt.creator}})
logger.warning(f'Unknown user {appt.user} in stored appointment {appt.iden} {appt.name}',

Check warning on line 782 in synapse/lib/agenda.py

View check run for this annotation

Codecov / codecov/patch

synapse/lib/agenda.py#L782

Added line #L782 was not covered by tests
extra={'synapse': {'iden': appt.iden, 'name': appt.name, 'user': appt.user}})
await self._markfailed(appt, 'unknown user')
return

locked = user.info.get('locked')
if locked:
logger.warning(f'Cron {appt.iden} {appt.name} failed because creator {user.name} is locked',
extra={'synapse': {'iden': appt.iden, 'name': appt.name, 'user': appt.creator,
logger.warning(f'Cron {appt.iden} {appt.name} failed because user {user.name} is locked',
extra={'synapse': {'iden': appt.iden, 'name': appt.name, 'user': appt.user,
'username': user.name}})
await self._markfailed(appt, 'locked user')
return

view = self.core.getView(iden=appt.view, user=user)
if view is None:
logger.warning(f'Unknown view {appt.view} in stored appointment {appt.iden} {appt.name}',
extra={'synapse': {'iden': appt.iden, 'name': appt.name, 'user': appt.creator,
extra={'synapse': {'iden': appt.iden, 'name': appt.name, 'user': appt.user,
'username': user.name, 'view': appt.view}})
await self._markfailed(appt, 'unknown view')
return
Expand Down
3 changes: 2 additions & 1 deletion synapse/lib/schemas.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,7 @@
'properties': {
'storm': {'type': 'string'},
'creator': {'type': 'string', 'pattern': s_config.re_iden},
'user': {'type': 'string', 'pattern': s_config.re_iden},
'iden': {'type': 'string', 'pattern': s_config.re_iden},
'view': {'type': 'string', 'pattern': s_config.re_iden},
'name': {'type': 'string'},
Expand Down Expand Up @@ -119,7 +120,7 @@
},
},
'additionalProperties': False,
'required': ['creator', 'storm'],
'required': ['creator', 'storm', 'user'],
'dependencies': {
'incvals': ['incunit'],
'incunit': ['incvals'],
Expand Down
Loading
Loading